Closed Bug 1495601 Opened 2 years ago Closed 2 years ago

jstests fails if it's executed without virtualenv


(Core :: JavaScript Engine, defect)

Not set





(Reporter: arai, Assigned: arai)




(1 file)

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

the following line fails because virtualenv is not created.
>     try:
>         from mozbuild.base import MozbuildObject, BuildEnvironmentNotFoundException

and it ends up raising error:
>         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
Attachment #9013463 - Flags: review?(sphink)
Attachment #9013463 - Flags: review?(sphink) → review+
Bug 1495601 - Do not raise error if is executed without virtualenv on non-automation. r=sfink
Created web-platform-tests PR for changes under testing/web-platform/tests
Closed: 2 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/ 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:
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 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 as requested by jgraham.
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
Backout by
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.
Closed: 2 years ago2 years ago
Resolution: --- → WORKSFORME
See Also: → 1509774
You need to log in before you can comment on or make changes to this bug.