Closed
Bug 543101
Opened 14 years ago
Closed 14 years ago
two windows-specific zipwriter test failures due to excess date stamp precision (test_asyncadd.js and test_sync.js)
Categories
(Core :: Networking: JAR, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.9.3a1
People
(Reporter: zwol, Assigned: zwol)
References
()
Details
(Keywords: regression)
Attachments
(1 file)
2.88 KB,
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
Whenever I push anything to the try server, I get two zipwriter test failures on Windows only. In the log, it appears to be four failures, but that's because each one gets reported twice. These are the relevant bits of the log: TEST-UNEXPECTED-FAIL | e:/builds/slave/win32-unit/mozilla/objdir/_tests/xpcshell/test_zipwriter/unit/test_asyncadd.js | 1264749650000 == 1264749650755 - See following stack: JS frame :: e:\builds\slave\win32-unit\mozilla\testing\xpcshell\head.js :: do_throw :: line 202 JS frame :: e:\builds\slave\win32-unit\mozilla\testing\xpcshell\head.js :: do_check_eq :: line 232 JS frame :: e:/builds/slave/win32-unit/mozilla/objdir/_tests/xpcshell/test_zipwriter/unit/test_asyncadd.js :: anonymous :: line 81 TEST-UNEXPECTED-FAIL | e:/builds/slave/win32-unit/mozilla/objdir/_tests/xpcshell/test_zipwriter/unit/test_sync.js | 1264749650000 == 1264749650755 - See following stack: JS frame :: e:\builds\slave\win32-unit\mozilla\testing\xpcshell\head.js :: do_throw :: line 202 JS frame :: e:\builds\slave\win32-unit\mozilla\testing\xpcshell\head.js :: do_check_eq :: line 232 JS frame :: e:/builds/slave/win32-unit/mozilla/objdir/_tests/xpcshell/test_zipwriter/unit/test_sync.js :: run_test :: line 84 Both failures are due to the same code construct: var source = do_get_file(DATA_DIR + TESTS[i].name); var entry = zipR.getEntry(TESTS[i].name); // ... do_check_eq(entry.lastModifiedTime / PR_USEC_PER_MSEC, source.lastModifiedTime); You can see from the failure line -- "1264749650000 == 1264749650755" -- that what's going on here is, the object returned by do_get_file has a lastModifiedTime with millisecond precision, whereas the object returned by zipR.getEntry has a lastModifiedTime with only second precision. The Zip format specification [<http://www.pkware.com/documents/casestudies/APPNOTE.TXT>] does not actually define the encoding of any of the (several) "last modification time" fields. Looking at the code in nsZipHeader.cpp leads me to believe that none of them carry fractional-second precision. Thus, these tests should be truncating both numbers to second precision before comparing them. I'll attach a patch to that effect shortly. I imagine that these tests only fail on Windows because Unix filesystems still don't generally support sub-second precision in their own timestamps. The rest of the zipfile unit tests are not subject to this problem because they force the use of a timestamp value that's a round number of seconds.
Assignee | ||
Comment 1•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Attachment #424315 -
Flags: review? → review?(dtownsend)
Comment 2•14 years ago
|
||
Why are these tests failing on try server but not on the main tinderbox?
Assignee | ||
Comment 3•14 years ago
|
||
I have no idea! The only thing I can think of is maybe the Windows boxes on the main tinderbox are configured differently and don't report sub-second timestamps for files, or maybe the source tree has been set up there so that all its files have timestamps that are an exact number of seconds. Do you have any idea who might know about that?
Comment 4•14 years ago
|
||
Someone from build, Ben perhaps
Comment 5•14 years ago
|
||
Perhaps this is an NTFS vs FAT thing
Assignee | ||
Comment 6•14 years ago
|
||
For what it's worth, other people's tryserver pushes are getting the same failure, e.g. http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264792687.1264804134.7050.gz http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264727946.1264737108.31425.gz http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1264725306.1264736272.21118.gz
Comment 7•14 years ago
|
||
Yeah I suspect the fix is the right one, would just like to make sure we have the full story here first.
Comment 8•14 years ago
|
||
Note that FAT only has 2 second timestamp resolution, AIUI.
Comment 10•14 years ago
|
||
The way I've heard it, the mozilla-central slaves are a mixture of FAT and NTFS, so if that was the difference we should see randomorange, not permaorange just on try.
Comment 11•14 years ago
|
||
Adding the test names to the summary to make it easier to find this bug.
Summary: two windows-specific zipwriter test failures due to excess date stamp precision → two windows-specific zipwriter test failures due to excess date stamp precision (test_asyncadd.js and test_sync.js)
Comment 12•14 years ago
|
||
AIUI, try slaves are completely separate from the other build slaves currently.
Comment 13•14 years ago
|
||
I reproduce these 2 failures on my local Windows.
Updated•14 years ago
|
Attachment #424315 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 14•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/0a1e074e9ddb
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 15•14 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20100203 SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4) V.Fixed, with patch manually applied. (on NTFS) PS: It looks like a regression from bug 525741.
Blocks: 525741
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Keywords: regression
Target Milestone: --- → mozilla1.9.3a1
Assignee | ||
Comment 16•14 years ago
|
||
Do you think it's worth bothering to put this on release branches?
Comment 17•14 years ago
|
||
Test-only changes have blanket approval for branch landing, so feel free to land it there. If it reduces the amount of orange, then it's a good thing.
Assignee | ||
Comment 18•14 years ago
|
||
Well, AFAIK it's only an orange on the try server. Has anyone seen it on the branches, is really what I was asking.
Assignee | ||
Comment 19•14 years ago
|
||
Hm, 525741 isn't on any branches at the moment, so really the question is if *that* (which does involve code changes) should be backported.
You need to log in
before you can comment on or make changes to this bug.
Description
•