Closed Bug 1342233 Opened 3 years ago Closed 3 years ago

unit-symbolstore.py fails in cross-OSX builds

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

If we enable 'make check' in cross-OSX builds, unit-symbolstore.py fails with a few errors:

[task 2017-02-23T18:39:36.020358Z] 18:39:36  WARNING - TEST-UNEXPECTED-FAIL | /home/worker/workspace/build/src/toolkit/crashreporter/tools/unit-symbolstore.py | TestCopyDebug.test_copy_debug_universal, line 212: 1 != 0
[task 2017-02-23T18:39:36.020567Z] 18:39:36     INFO - FAIL: test_copy_debug_universal (__main__.TestCopyDebug)
[task 2017-02-23T18:39:36.020924Z] 18:39:36     INFO - Traceback (most recent call last):
[task 2017-02-23T18:39:36.021175Z] 18:39:36     INFO -   File "/home/worker/workspace/build/src/toolkit/crashreporter/tools/unit-symbolstore.py", line 212, in test_copy_debug_universal
[task 2017-02-23T18:39:36.021361Z] 18:39:36     INFO -     self.assertEqual(1, len(copied))
[task 2017-02-23T18:39:36.021541Z] 18:39:36     INFO - AssertionError: 1 != 0
[task 2017-02-23T18:39:36.021846Z] 18:39:36  WARNING - TEST-UNEXPECTED-FAIL | /home/worker/workspace/build/src/toolkit/crashreporter/tools/unit-symbolstore.py | TestExclude.test_exclude_filenames, line 152: Lists differ: [] != ['abc/bar.so', 'bar.so', 'def/...
[task 2017-02-23T18:39:36.022042Z] 18:39:36     INFO - FAIL: test_exclude_filenames (__main__.TestExclude)
[task 2017-02-23T18:39:36.022227Z] 18:39:36     INFO - Traceback (most recent call last):
[task 2017-02-23T18:39:36.022486Z] 18:39:36     INFO -   File "/home/worker/workspace/build/src/toolkit/crashreporter/tools/unit-symbolstore.py", line 152, in test_exclude_filenames
[task 2017-02-23T18:39:36.022662Z] 18:39:36     INFO -     self.assertEqual(processed, expected)
[task 2017-02-23T18:39:36.022867Z] 18:39:36     INFO - AssertionError: Lists differ: [] != ['abc/bar.so', 'bar.so', 'def/...
[task 2017-02-23T18:39:36.023054Z] 18:39:36     INFO - Second list contains 3 additional elements.
[task 2017-02-23T18:39:36.023232Z] 18:39:36     INFO - First extra element 0:
[task 2017-02-23T18:39:36.023405Z] 18:39:36     INFO - abc/bar.so
[task 2017-02-23T18:39:36.023580Z] 18:39:36     INFO - - []
[task 2017-02-23T18:39:36.023775Z] 18:39:36     INFO - + ['abc/bar.so', 'bar.so', 'def/bar.so']
[task 2017-02-23T18:39:36.024069Z] 18:39:36  WARNING - TEST-UNEXPECTED-FAIL | /home/worker/workspace/build/src/toolkit/crashreporter/tools/unit-symbolstore.py | TestExclude.test_exclude_wildcard, line 130: Lists differ: [] != ['abc/xyz.so', 'bar.so', 'def/...
[task 2017-02-23T18:39:36.024262Z] 18:39:36     INFO - FAIL: test_exclude_wildcard (__main__.TestExclude)
[task 2017-02-23T18:39:36.024445Z] 18:39:36     INFO - Traceback (most recent call last):
[task 2017-02-23T18:39:36.024709Z] 18:39:36     INFO -   File "/home/worker/workspace/build/src/toolkit/crashreporter/tools/unit-symbolstore.py", line 130, in test_exclude_wildcard
[task 2017-02-23T18:39:36.024905Z] 18:39:36     INFO -     self.assertEqual(processed, expected)
[task 2017-02-23T18:39:36.025106Z] 18:39:36     INFO - AssertionError: Lists differ: [] != ['abc/xyz.so', 'bar.so', 'def/...
[task 2017-02-23T18:39:36.025294Z] 18:39:36     INFO - Second list contains 3 additional elements.
[task 2017-02-23T18:39:36.025472Z] 18:39:36     INFO - First extra element 0:
[task 2017-02-23T18:39:36.025644Z] 18:39:36     INFO - abc/xyz.so
[task 2017-02-23T18:39:36.025818Z] 18:39:36     INFO - - []
[task 2017-02-23T18:39:36.026012Z] 18:39:36     INFO - + ['abc/xyz.so', 'bar.so', 'def/asdf.so']

We'll probably need to fix these somehow (instead moving them to a separate task) since the unit-symbolstore.py relies on an objdir.
There are two things here:
The first is that some of the tests are checking the current platform, which is going to make them fail because they also use build config so that's not going to match up in a cross-compile.

The second is that there's at least one test in there that actually runs dump_syms and checks the output, and it might not do the right thing on a cross build. If we want to run that test it will have to actually run on the build machine.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> The first is that some of the tests are checking the current platform, which
> is going to make them fail because they also use build config so that's not
> going to match up in a cross-compile.

These were pretty straightforward to fix - just changing platform.system() to checking OS_TARGET instead (along with switching Windows/Microsoft to WINNT) gets them to work.

> The second is that there's at least one test in there that actually runs
> dump_syms and checks the output, and it might not do the right thing on a
> cross build. If we want to run that test it will have to actually run on the
> build machine.

This almost works out of the box, with the caveat that the cross build version of dist/host/bin/dump_syms requires path-to-clang/lib to be in LD_LIBRARY_PATH. This works fine if run from 'make check' because we add that to LD_LIBRARY_PATH from the mozconfig: https://dxr.mozilla.org/mozilla-central/source/build/macosx/cross-mozconfig.common#14

However if you run 'mach python-test toolkit/crashreporter/tools' directly, we don't pull in the LD_LIBRARY_PATH and the test fails. (Overriding LD_LIBRARY_PATH in the command-line makes the test pass too.) This could probably be fixed by using the linker to stash clang/lib in the rpath, but I don't think it's really worth it for this. Mostly I just want to address this issue so that we can ensure we're not missing any valuable testing by not turning on 'make check' in the cross build.
Assignee: nobody → mshal
Comment on attachment 8843405 [details]
Bug 1342233 - Skip hg test in unit-symbolstore.py for git users;

https://reviewboard.mozilla.org/r/117172/#review124910
Attachment #8843405 - Flags: review?(ted) → review+
Comment on attachment 8843406 [details]
Bug 1342233 - Fix unit-symbolstore.py in cross-OSX builds;

https://reviewboard.mozilla.org/r/117174/#review124914

::: toolkit/crashreporter/tools/unit-symbolstore.py:512
(Diff revision 1)
>          expected_output = ''.join(mk_output(expected_files))
>          symbol_file = os.path.join(self.symboldir,
>                                     file_id[1], file_id[0], file_id[1] + '.sym')
>          self.assertEqual(open(symbol_file, 'r').read(), expected_output)
>  
>  class TestFunctional(HelperMixin, unittest.TestCase):

FWIW, we could split `TestFunctional` out to a separate file, and the rest of the tests could be run from a test package. This test does need to run with an actual built objdir, but it should be the only one.
Attachment #8843406 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> Comment on attachment 8843406 [details]
> >  class TestFunctional(HelperMixin, unittest.TestCase):
> 
> FWIW, we could split `TestFunctional` out to a separate file, and the rest
> of the tests could be run from a test package. This test does need to run
> with an actual built objdir, but it should be the only one.

Good point. I think we should probably discuss whether or not it is worthwhile to continue pulling things out of 'make check', and if we decide that it is we can move out most of these.
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bed5fd619e8d
Skip hg test in unit-symbolstore.py for git users; r=ted
https://hg.mozilla.org/integration/autoland/rev/7c4d5f5b99a8
Fix unit-symbolstore.py in cross-OSX builds; r=ted
https://hg.mozilla.org/mozilla-central/rev/bed5fd619e8d
https://hg.mozilla.org/mozilla-central/rev/7c4d5f5b99a8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.