1654589 - Move libpref Python unit test to mozbuild suite

Status: RESOLVED FIXED

(Firefox Build System :: General, task)

Comment 1

[Ricky Stewart]

Attachment: Bug 1654589 - Move `libpref` Python unit test to `mozbuild` suite

It could go into its own test suite, but it 1) depends on mozbuild code, so the mozbuild suite as well as this new suite would be running on any push that touches mozbuild code anyway, and 2) this is code that runs during the build, so it's not out of place.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e890ed131173
Move `libpref` Python unit test to `mozbuild` suite r=froydnj

Comment 3

[Atila Butkovits]

Backed outfor bustage at Punion_Uncomparable.ipdl.

Backout link: https://hg.mozilla.org/integration/autoland/rev/88f613fb27a4e31f45c78d81596a58d32f18d820

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=YsRGrMZwQXalQ7voCHOnMg.0&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Crunnable&searchStr=linux%2Cx64%2Casan%2Cbuild-linux64-asan%2Fopt%2Cbo&revision=e890ed131173abf9900a68cbc56840cc6d4349c2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310751208&repo=autoland&lineNumber=67597

Comment 4

[Simon Giesecke [:sg] [he/him]]

For some reason, this patch causes the IPDL tests to be run on asan/tsan build jobs which was not the case before, and they have some issues. https://phabricator.services.mozilla.com/D84656 is intended to fix that. I can imagine the patch from this bug can then reland without modification.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9168828ce9e9
Move `libpref` Python unit test to `mozbuild` suite r=froydnj

Comment 6

[Ricky Stewart]

Giving it another shot after a tweak to the Makefile.

Comment 7

[Dorel Luca [:dluca]]

Backed out changeset 9168828ce9e9 (bug 1654589) as requested by dev for breaking fuzzy on a build. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310876661&repo=autoland&lineNumber=62365

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=9168828ce9e9e74107bd8a64324981e6d5c70ea7

Backout:
https://hg.mozilla.org/integration/autoland/rev/074ba1c126005895f0520596c3f1d237882eb7df

Comment 8

[Ricky Stewart]

Simon, I'm seeing the same behavior for fuzzing builds with my newest patch, see: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=FN8_-ZmCTbCvDmY8XrTBGA.0&fromchange=db80fd6939332103f7062b545cf88e903c21fc6c

What should we do about this? Is there a corresponding fix you can put in for the IPDL test failures here?

Comment 9

[Andrew McCreight [:mccr8]]

(In reply to Ricky Stewart from comment #8)

What should we do about this? Is there a corresponding fix you can put in for the IPDL test failures here?

It looks like ipc/ipdl/test/ipdl/sync-messages.ini needs to be updated to add the new test protocols.

Comment 10

[Ricky Stewart]

Thanks, I requested review from you on the patch for that change.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1245f6702e78
Move `libpref` Python unit test to `mozbuild` suite r=froydnj,mccr8

Comment 12

[Simon Giesecke [:sg] [he/him]]

(In reply to Andrew McCreight [:mccr8] from comment #9)

(In reply to Ricky Stewart from comment #8)

What should we do about this? Is there a corresponding fix you can put in for the IPDL test failures here?

It looks like ipc/ipdl/test/ipdl/sync-messages.ini needs to be updated to add the new test protocols.

Oh, sorry I missed this.

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1245f6702e78

Comment 14

[Ricky Stewart]

Here's a recent mozilla-central build log: https://firefoxci.taskcluster-artifacts.net/W0Ub_MqCQReEeSmJ4gSm2Q/0/public/logs/live_backing.log

[task 2020-07-28T21:59:55.961Z] 21:59:55     INFO -  check> ...............................F..............................................................................................................
[task 2020-07-28T21:59:55.961Z] 21:59:55     INFO -  check> ======================================================================
[task 2020-07-28T21:59:55.961Z] 21:59:55     INFO -  check> FAIL: test (__main__.OkTestCase)
[task 2020-07-28T21:59:55.961Z] 21:59:55     INFO -  check> OkTestCase test of "/builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/ok/Punion_Comparable.ipdl"
[task 2020-07-28T21:59:55.961Z] 21:59:55     INFO -  check> ----------------------------------------------------------------------
[task 2020-07-28T21:59:55.962Z] 21:59:55    ERROR -  check> Traceback (most recent call last):
[task 2020-07-28T21:59:55.962Z] 21:59:55     INFO -  check>   File "/builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/runtests.py", line 16, in test
[task 2020-07-28T21:59:55.962Z] 21:59:55     INFO -  check>     self.checkPassed()
[task 2020-07-28T21:59:55.962Z] 21:59:55     INFO -  check>   File "/builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/runtests.py", line 43, in checkPassed
[task 2020-07-28T21:59:55.962Z] 21:59:55     INFO -  check>     self.assertTrue(self.compile.ok(), self.mkFailMsg())
[task 2020-07-28T21:59:55.962Z] 21:59:55     INFO -  check> AssertionError: False is not true :
[task 2020-07-28T21:59:55.963Z] 21:59:55     INFO -  check> ### Command: /builds/worker/workspace/obj-build/_virtualenvs/init_py3/bin/python /builds/worker/checkouts/gecko/ipc/ipdl/ipdl.py --sync-msg-list=/builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/sync-messages.ini --msg-metadata=/builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/message-metadata.ini -I /builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/ok -d /tmp/ipdl_unit_testel3gba9u /builds/worker/checkouts/gecko/ipc/ipdl/test/ipdl/ok/Punion_Comparable.ipdl
[task 2020-07-28T21:59:55.963Z] 21:59:55     INFO -  check> ### stderr:
[task 2020-07-28T21:59:55.963Z] 21:59:55     INFO -  check> Error: Sync IPC message Punion_Comparable::test not found, it appears to be fixed.
[task 2020-07-28T21:59:55.963Z] 21:59:55     INFO -  check> Please remove it from sync-messages.ini.

This is apparently due to the changes made in this patch. Two questions:

1. What does this mean? Does the Punion_Comparable::test protocol need to be added for some builds and not for others or something?

2. This is a log from a successful build. Should builds be failing if these tests fail?

Comment 15

[Andrew McCreight [:mccr8]]

Looking at the code, it seems like it is complaining that this is marked as a sync message, but it isn't a sync message I saw. I assume this is for the sync message checking for the IPDL we use in the browser, and these unit tests are not included. There's a comment in checkFixedSyncMessages that says it is supposed to ignore test messages, and in fact we have some existing sync messages, so I guess it must work under some cases, but I'm not sure how. I'm unfortunately not familiar with the sync messages checker so I'll have to dig around some more.

Comment 16

[Andrew McCreight [:mccr8]]

Oh, sorry, this is simpler than I thought. This is running during the unit test, and in fact Punion_Comparable::test does not seem to exist, so you should just delete it from the sync-messages.ini list. I'm not sure why it got added. I'll look over the list.

As for you second question, it would be great if these tests were run, but the current limbo state where we run them but don't actually turn the test suite orange seems bad.

Comment 17

[Andrew McCreight [:mccr8]]

It looks like the two messages that should have been added to that list are Punion_Comparable::Msg and PStructComparable::test. So, just remove Punion_Comparable::test. Sorry, I should have looked your patch over more when I reviewed it.

Comment 18

[Ricky Stewart]

Whoops, okay, this is my bad then. Sending out a patch now :)