TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_cache2-29e-non-206-response.js and TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_cache2-29d-concurrent_read_half-corrupted-206.js

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jorgk, Assigned: aleth)

Tracking

Trunk
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(2 attachments)

First seen 2016-07-20:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=7f04347eceeee06ac4a3f01b37fe5dcfedb4c791
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=42385

TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_cache2-29e-non-206-response.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_cache2-29d-concurrent_read_half-corrupted-206.js | xpcshell return code: 0
Honza, can you take a look?
Flags: needinfo?(dd.mozilla) → needinfo?(honzab.moz)
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #1)
> http://hg.mozilla.org/mozilla-central/log/tip/netwerk/test/unit/test_cache2-
> 29d-concurrent_read_half-corrupted-206.js
> http://hg.mozilla.org/mozilla-central/log/tip/netwerk/test/unit/test_cache2-
> 29e-non-206-response.js
> New in bug 1274818.
> 
> Dragana, any hint why this wouldn't work as part of the TB Xpcshell tests?

Probably because TB is using the old cache that is incapable of concurrent reading?

Let's do the "don't run when new cache is off" trick here..
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Flags: needinfo?(honzab.moz)
Thanks!!
- disables the tests when the old cache is active (TB is not using cache2)
- test_cache2-29e-non-206-response.js given a better name
Attachment #8773193 - Flags: review?(dd.mozilla)
Attachment #8773193 - Flags: review?(dd.mozilla) → review+
Thanks!

no need for a try, this will just effect TB anyway.
Keywords: checkin-needed
Thanks, great. This has already bitten us in bug 1277354. We should really switch to cache2, see bug 1021843.
Component: General → Networking
Product: Thunderbird → Core
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/730a8fbf67f9
Disable some HTTP cache tests when the old backend is used, r=dragana
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/730a8fbf67f9
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Thanks for fix, however, we're still seeing the test failure:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=6dcba5e46ad7f88dae81ec7ebd8a2c9c02f98caa
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=42693

Am I missing something?
Flags: needinfo?(honzab.moz)
Flags: needinfo?(aleth)
(In reply to Jorg K (GMT+2, PTO during summer) from comment #10)
> Thanks for fix, however, we're still seeing the test failure:
> https://treeherder.mozilla.org/#/jobs?repo=comm-
> central&revision=6dcba5e46ad7f88dae81ec7ebd8a2c9c02f98caa
> https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=42693
> 
> Am I missing something?

Has this comm-central pulled the m-c changeset containing the fix?
Flags: needinfo?(honzab.moz) → needinfo?(mozilla)
This fixes the remaining two failures for me.
Attachment #8774709 - Flags: review?(honzab.moz)
Assignee: honzab.moz → aleth
Flags: needinfo?(aleth)
(In reply to Honza Bambas (:mayhemer) from comment #11)
> Has this comm-central pulled the m-c changeset containing the fix?
Sure, before these tests failed:
netwerk/test/unit/test_cache2-29e-non-206-response.js
netwerk/test/unit/test_cache2-29d-concurrent_read_half-corrupted-206.js

With your fix and the renamed test, these fail:
netwerk/test/unit/test_cache2-29d-concurrent_read_half-corrupted-206.js
netwerk/test/unit/test_cache2-29e-concurrent_read_half-non-206-response.js

Aleth has already prepared a patch.
Flags: needinfo?(mozilla)
Comment on attachment 8774709 [details] [diff] [review]
Disable some HTTP cache tests when the old backend is used, followup to fix two additional failures

Review of attachment 8774709 [details] [diff] [review]:
-----------------------------------------------------------------

Oh!!  I see now what I've screwed up!  Yes, the "d" test had to be disabled, not the "c" test.

Please, yet enable again the "c" test (remove my change from it), I think it works on comm-central well.

Thanks!

::: netwerk/test/unit/test_cache2-29e-concurrent_read_half-non-206-response.js
@@ +57,5 @@
>  {
> +  // Static check
> +  do_check_true(responseBody.length > 1024);
> +
> +  do_get_profile();

we need profile to read the correct prefs, right?  I almost forgot about this..  

(Really, we should get rid of the pref, and never use the old backend for content HTTP caching...  These will pop-up again and again...)
Attachment #8774709 - Flags: review?(honzab.moz) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/37e371c92c11c20fa0a8dd32f4f6ecee7aea7b49
Bug 1288327 - Disable some HTTP cache tests when the old backend is used, followup to fix remaining failures. r=mayhemer
Pushed by aleth@instantbird.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/37e371c92c11
Disable some HTTP cache tests when the old backend is used, followup to fix remaining failures. r=mayhemer
(In reply to Honza Bambas (:mayhemer) from comment #14)
> (Really, we should get rid of the pref, and never use the old backend for
> content HTTP caching...  These will pop-up again and again...)

Yeah, and it's time for TB to make the switch...
(In reply to aleth [:aleth] from comment #17)
> Yeah, and it's time for TB to make the switch...
It's on my "to do" list, see bug 1021843. Sadly, I know nothing about it, so I might not be the best candidate.
You need to log in before you can comment on or make changes to this bug.