Closed Bug 1654589 Opened 2 months ago Closed 2 months ago

Move libpref Python unit test to mozbuild suite

Categories

(Firefox Build System :: General, task)

task

Tracking

(firefox80 fixed)

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: rstewart, Assigned: rstewart)

References

Details

Attachments

(1 file)

No description provided.

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.

Assignee: nobody → rstewart
Status: NEW → ASSIGNED
Blocks: 1210759
Pushed by rstewart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e890ed131173
Move `libpref` Python unit test to `mozbuild` suite r=froydnj

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.

Pushed by rstewart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9168828ce9e9
Move `libpref` Python unit test to `mozbuild` suite r=froydnj

Giving it another shot after a tweak to the Makefile.

Flags: needinfo?(rstewart)

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?

Flags: needinfo?(sgiesecke)

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

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

Flags: needinfo?(sgiesecke)
Flags: needinfo?(rstewart)
Pushed by rstewart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1245f6702e78
Move `libpref` Python unit test to `mozbuild` suite r=froydnj,mccr8

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

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

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?

Flags: needinfo?(continuation)

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.

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.

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.

Flags: needinfo?(continuation)
Regressions: 1655993

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

You need to log in before you can comment on or make changes to this bug.