Closed Bug 1495601 Opened 6 years ago Closed 6 years ago

jstests fails if it's executed without virtualenv

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

after bug 1305743, running jstests.py locally fails because it cannot find mozinfo.json.

the following line fails because virtualenv is not created.
https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/testing/mozbase/mozinfo/mozinfo/mozinfo.py#245
>     try:
>         from mozbuild.base import MozbuildObject, BuildEnvironmentNotFoundException

and it ends up raising error:
https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/testing/mozbase/mozinfo/mozinfo/mozinfo.py#267
>         raise EnvironmentError('{}: could not find any mozinfo.json.'.format(__name__))

as far as I can see, the above import has been failing for a long time for local testing, but it was silently ignored.

anyway, for local testing, the failure won't be fatal, and we should pass 'raise_exception'=False to suppress the fatal error.
for now I disabled the error on non-automation.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #9013463 - Flags: review?(sphink)
Attachment #9013463 - Flags: review?(sphink) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6568853848ac62384a094e521965e101978b1f62
Bug 1495601 - Do not raise error if jstests.py is executed without virtualenv on non-automation. r=sfink
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13309 for changes under testing/web-platform/tests
https://hg.mozilla.org/mozilla-central/rev/6568853848ac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
This broke unit tests upstream.
Flags: needinfo?(lhansen)
Flags: needinfo?(james)
I don't know anything about the patch but this seems very related to the problem I ran into yesterday when testing on a system that was not also a build system, so preserving the functionality of the change would be welcome.
Flags: needinfo?(lhansen)
maybe testing/mozharness/mozinfo/mozinfo.py should also be updated?
it complains that find_and_update_from_json doesn't accept keyword arguments,
and what I can find is that another definition of find_and_update_from_json there, which I haven't hit while fixing this bug.
if the issue is from the outdated code in mozharness, bug 1372239 will fix it.
No, the problem is that Bug 1305743 changed mozinfo API and behaviour in a backwards-incompatible way. That behaviour change doesn't make sense in all cases (e.g. the case in this bug, or any other case where wpt is run against something other than a gecko built from source). So this bug changed the wpt harness to opt in to the old behaviour. That opt-in requires API surface that doesn't exist in the released mozinfo, and therefore breaks wpt on GitHub which is using the released version.

I'm not sure what the best fix is; I tend to think that we should revert the change in Bug 1305743 and look for an approach that doesn't involve so many breaking changes.
Flags: needinfo?(james)
Flags: needinfo?(gbrown)
Flags: needinfo?(egao)
Flags: needinfo?(ahal)
Yeah, let's keep it backwards compatible. Rather than revert, we can just change the default of `raise_exception` to False:
https://searchfox.org/mozilla-central/source/testing/mozbase/mozinfo/mozinfo/mozinfo.py#257
Flags: needinfo?(ahal)
I advocated for the incompatible change, reasoning that callers should not expect mozinfo.find_and_update_from_json(<dirs-without-mozinfo>) to "succeed". I'm hinting that failing callers should re-examine their use case, but in light of the trouble seen here, I agree we should change the default of raise_exception to False.
Flags: needinfo?(gbrown)
I agreed with the reasoning of :ahal and :gbrown when I refactored the behavior. However, whatever the intended result the actual result differed.

I have reopened the Phabricator diff for this issue at https://phabricator.services.mozilla.com/D6860. If the call to kwargs cannot find a raise_exception key, it will now default to False.

Unit tests failed when I made the change (as I would expect) and so its behavior was also changed to expect None to be raised by default.
Flags: needinfo?(egao)
This was backed out together with https://bugzilla.mozilla.org/show_bug.cgi?id=1305743#c17 as requested by jgraham.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
Backout by csabou@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/317b91c1fbd5
Backed out changeset 6568853848ac on request from jgraham for causing issues with mozinfo.json. a=backout
Upstream PR was closed without merging
closing given bug 1305743 is backed out and it works without any change now.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → WORKSFORME
See Also: → 1509774
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: