spurious failure in test_web.Web.test_welcome* with Nevow 0.12.0/Twisted 15.5.0 #2663

Closed
opened 2015-12-21 22:13:48 +00:00 by daira · 10 comments
daira commented 2015-12-21 22:13:48 +00:00
Owner

These tests:

allmydata.test.test_web.Web.test_welcome
allmydata.test.test_web.Web.test_welcome_page_mkdir_button

are failing with Nevow 0.12.0 and/or Twisted 15.5.0 (at least I think that's why), because a test is looking for:

<img src="img/connected-not-configured.png" alt="Not Configured" />

but actually seeing

<img alt="Not Configured" src="img/connected-not-configured.png" />

https://travis-ci.org/tahoe-lafs/tahoe-lafs/jobs/98205053

These tests: ``` allmydata.test.test_web.Web.test_welcome allmydata.test.test_web.Web.test_welcome_page_mkdir_button ``` are failing with Nevow 0.12.0 and/or Twisted 15.5.0 (at least I think that's why), because a test is looking for: ``` <img src="img/connected-not-configured.png" alt="Not Configured" /> ``` but actually seeing ``` <img alt="Not Configured" src="img/connected-not-configured.png" /> ``` <https://travis-ci.org/tahoe-lafs/tahoe-lafs/jobs/98205053>
tahoe-lafs added the
code-frontend-web
normal
defect
1.10.2
labels 2015-12-21 22:13:48 +00:00
tahoe-lafs changed title from failure in test_web.Web.test_welcome* with Nevow 0.12.0/Twisted 15.5.0 to spurious failure in test_web.Web.test_welcome* with Nevow 0.12.0/Twisted 15.5.0 2015-12-21 22:14:29 +00:00
daira commented 2015-12-21 22:17:25 +00:00
Author
Owner

Actually it appears that 9 tests are failing.

Actually it appears that 9 tests are failing.
daira commented 2016-01-01 00:37:06 +00:00
Author
Owner

This is a release blocker because it prevents us from being able to tell whether the remainder of those tests would pass.

This is a release blocker because it prevents us from being able to tell whether the remainder of those tests would pass.
tahoe-lafs added
major
and removed
normal
labels 2016-01-01 00:37:06 +00:00
Author
Owner

This is caused by this change in Nevow; https://github.com/twisted/nevow/commit/f88939ac957d6c2bfdcf5e91edd2baf1c4ae3c5d

Attributes are now sorted. How should we fix this? Since pip-install users (which is what Travis CI does) will get 0.12, but Debian and Ubuntu users get 0.10 or 0.11, we probably want to support both.

An easy fix would be to require Nevow 0.11, but this is obviously not a long-term solution.

Another option would be to change all of the tests to actually parse the rendered HTML. This could require minimal changes to the tests themselves; simply replacing failUnlessIn with (the as-yet unwritten but easy to write) failUnlessXmlIn. I don't think we currently have an HTML parser in our dependencies, though.

This is caused by this change in Nevow; <https://github.com/twisted/nevow/commit/f88939ac957d6c2bfdcf5e91edd2baf1c4ae3c5d> Attributes are now sorted. How should we fix this? Since pip-install users (which is what Travis CI does) will get 0.12, but Debian and Ubuntu users get 0.10 or 0.11, we probably want to support both. An easy fix would be to require Nevow 0.11, but this is obviously not a long-term solution. Another option would be to change all of the tests to actually parse the rendered HTML. This could require minimal changes to the tests themselves; simply replacing `failUnlessIn` with (the as-yet unwritten but easy to write) `failUnlessXmlIn`. I don't think we currently have an HTML parser in our dependencies, though.
daira commented 2016-01-15 18:40:55 +00:00
Author
Owner

I think we can fix this with minimal changes, by making sure that the order in which we present attributes to Nevow is already sorted. Then the tests can continue to just check for a single order (the sorted one).

I think we can fix this with minimal changes, by making sure that the order in which we present attributes to Nevow is already sorted. Then the tests can continue to just check for a single order (the sorted one).
Author
Owner

Unfortunately I don't think that will work.

I think there are two ways attributes get there: via <n:attr> tags in templates (like this) and via named arguments in the call to Nevow's T function (like this). I'm pretty sure that in both cases the old Nevow does not let us control the order either: at least in the T case the attributes are rendered in dictionary order (via iteritems()) and from the Nevow commit in my previous comment I suspect attributes from <n:attr> tags are too.

I actually tried the crazy idea of passing a subclass of dict which sorts its iteritems to see if a function like T with ** in its signature would actually receive my special dictionary... but (unsurprisingly) when I call f(**d) the dictionary f receives is not an instance of my class.

Unfortunately I don't think that will work. I think there are two ways attributes get there: via `<n:attr>` tags in templates (like [this](https://github.com/tahoe-lafs/tahoe-lafs/blob/master/src/allmydata/web/welcome.xhtml#L141)) and via named arguments in the call to Nevow's `T` function (like [this](https://github.com/tahoe-lafs/tahoe-lafs/blob/master/src/allmydata/web/root.py#L400)). I'm pretty sure that in both cases the old Nevow does not let us control the order either: at least in the `T` case the attributes are rendered in dictionary order (via `iteritems()`) and from the Nevow commit in my previous comment I suspect attributes from `<n:attr>` tags are too. I actually tried the crazy idea of passing a subclass of `dict` which sorts its `iteritems` to see if a function like `T` with `**` in its signature would actually receive my special dictionary... but (unsurprisingly) when I call `f(**d)` the dictionary `f` receives is not an instance of my class.
warner commented 2016-01-28 21:41:26 +00:00
Author
Owner

I think the tests are being too picky.. we can get most of the same coverage by looking for shorter strings. Also, is the output split into separate lines in a useful way? We could have the test look for the line that shows the expected nickname, then grep just that line for the other attributes.

Worst case we res.split("</div>") and then find the section we want in the resulting list. Full HTML parsing is hard, but I don't think we need it to check what we need from this test.

I think the tests are being too picky.. we can get most of the same coverage by looking for shorter strings. Also, is the output split into separate lines in a useful way? We could have the test look for the line that shows the expected nickname, then grep just that line for the other attributes. Worst case we `res.split("</div>")` and then find the section we want in the resulting list. Full HTML parsing is hard, but I don't think we need it to check what we need from this test.
Author
Owner

OK, I'm working on this now and expect to have a ,,ZA̡͊͠͝LGΌ,, ^is not rè̑ͧ̌aͨl̘̝̙̃ͤ͂̾̆^ regexp-based fix soon.

OK, I'm working on this now and expect to have a ,,ZA̡͊͠͝LGΌ,, ^is not rè̑ͧ̌aͨl̘̝̙̃ͤ͂̾̆^ regexp-based fix soon.
Author
Owner
(https://github.com/tahoe-lafs/tahoe-lafs/pull/232)
Leif Ryge <leif@synthesize.us> commented 2016-01-31 18:59:30 +00:00
Author
Owner

In 55fdbaa/trunk:

Make tests work with both Nevow 0.11 and 0.12

closes #2663
In [55fdbaa/trunk](/tahoe-lafs/trac-2024-07-25/commit/55fdbaa3a23c3e038a62d73af72cee1bc9dbae29): ``` Make tests work with both Nevow 0.11 and 0.12 closes #2663 ```
tahoe-lafs added the
fixed
label 2016-01-31 18:59:30 +00:00
Leif Ryge <leif@synthesize.us> closed this issue 2016-01-31 18:59:30 +00:00
warner commented 2016-03-22 05:03:36 +00:00
Author
Owner

Milestone renamed

Milestone renamed
tahoe-lafs added this to the 1.11.0 milestone 2016-03-22 05:03:36 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/trac-2024-07-25#2663
No description provided.