1342233 - unit-symbolstore.py fails in cross-OSX builds

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Michael Shal [:mshal]]

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.

Comment 1

[(not currently active) Ted Mielczarek]

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.

Comment 2

[Michael Shal [:mshal]]

(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.

Comment 3

[Michael Shal [:mshal]]

Attachment: Bug 1342233 - Skip hg test in unit-symbolstore.py for git users;

Review commit: https://reviewboard.mozilla.org/r/117172/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/117172/

Comment 4

[Michael Shal [:mshal]]

Attachment: Bug 1342233 - Fix unit-symbolstore.py in cross-OSX builds;

Review commit: https://reviewboard.mozilla.org/r/117174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/117174/

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8843405 [details]
Bug 1342233 - Skip hg test in unit-symbolstore.py for git users;

https://reviewboard.mozilla.org/r/117172/#review124910

Comment 6

[(not currently active) Ted Mielczarek]

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.

Comment 7

[Michael Shal [:mshal]]

(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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bed5fd619e8d
https://hg.mozilla.org/mozilla-central/rev/7c4d5f5b99a8