Make failure to find mozinfo.json a fatal error

RESOLVED FIXED in Firefox 65


3 years ago
7 months ago


(Reporter: gps, Assigned: egao)


Version 3
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)



(2 attachments, 1 obsolete attachment)



3 years ago
In bug 1296397, I ran into issues where WPT was running in automation without a loaded mozinfo.json because the path to mozinfo.json changed. This caused a number of head-scratching test failures because it wasn't obvious that mozinfo.json wasn't loaded.

The code in mozinfo.find_and_update_from_json() currently no-ops if no mozinfo.json could be loaded. Since mozinfo.json is kinda critical to ensure tests run properly, we should consider changing it to fail if it can't find a mozinfo.json.

Alternatively, callers of this function relying on a mozinfo.json (such as automation) should raise if it returns None.

Comment 1

9 months ago
Try run of a proposed change:
Assignee: nobody → egao

Comment 2

9 months ago
Changes are causing two jobs to fail:

- linux x64 opt python 2 unit tests source-test-python-mozbase-py2 py2(mb).
- linux x64 opt python 3 unit tests source-test-python-mozbase-py3 py3(mb).

Unsure if this is the intended side effect of raising an explicit error instead of no-oping. An alternative explanation is that by default my MOZCONFIG contains --artifact flag, so these builds fail since it is expecting a full build to be performed prior to running the test.

:gbrown: What do you think? Is a more extensive try run needed to gauge further possible impact on test suite?
Flags: needinfo?(gbrown)
I don't think your MOZCONFIG would have an effect here.

Those failures look to be a result of your change:

[task 2018-09-25T16:14:34.319Z] Traceback (most recent call last):
[task 2018-09-25T16:14:34.319Z]   File "/builds/worker/checkouts/gecko/build/", line 39, in <module>
[task 2018-09-25T16:14:34.319Z]     sys.exit(gen_test_backend())
[task 2018-09-25T16:14:34.319Z]   File "/builds/worker/checkouts/gecko/build/", line 30, in gen_test_backend
[task 2018-09-25T16:14:34.319Z]     emitter = TreeMetadataEmitter(config)
[task 2018-09-25T16:14:34.319Z]   File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/frontend/", line 114, in __init__
[task 2018-09-25T16:14:34.319Z]     mozinfo.find_and_update_from_json(config.topobjdir)
[task 2018-09-25T16:14:34.319Z]   File "/builds/worker/checkouts/gecko/testing/mozbase/mozinfo/mozinfo/", line 266, in find_and_update_from_json
[task 2018-09-25T16:14:34.319Z]     raise IOError
[task 2018-09-25T16:14:34.319Z] IOError
[task 2018-09-25T16:14:34.332Z] /builds/worker/checkouts/gecko/build/ recipe for target 'backend.TestManifestBackend' failed
[task 2018-09-25T16:14:34.332Z] make: *** [backend.TestManifestBackend] Error 1

Does that mean that this find_and_update_from_json call normally no-ops in this test? Maybe you should re-run py2/3(mb) on try without your change but with some additional logging to verify.
Flags: needinfo?(gbrown)
Mozinfo was sort of designed to be unaware of things like "objdir" and "mozilla-central". It's definitely used by things that live outside of the tree as well. So the best place to fix this is likely in the test harnesses (e.g the 'update_mozinfo' function in mochitest).

The downside to that location is that we have to implement this for every test harness though (and new harnesses might omit this). Maybe the `find_and_update_from_json` function could take a raises parameter (which consumers that want an error can set).

Comment 5

9 months ago
After running through several different combinations locally and on try-server:

- replacing the | return None | call with | raise <Exception> | will cause py2/py3(mb) tests to fail on try-server.
- tested locally, the above change does not cause test failure in any scenario.

The failure location on try-server is in the test setup phase:

Build configuration changed. Regenerating backend.
[task 2018-09-25T20:24:43.302Z] No build detected, test metadata may be incomplete.
[task 2018-09-25T20:24:43.302Z] paths is: [u'/builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/mozinfo.json']
[task 2018-09-25T20:24:43.302Z] path has mozinfo: [False]
[task 2018-09-25T20:24:43.302Z] valid files: []

In the above snippet, debug messages I added are visible.

The following is a snippet from a try run where only debug messages were added:

[task 2018-09-25T19:26:04.428Z] Build configuration changed. Regenerating backend.
[task 2018-09-25T19:26:04.910Z] No handlers could be found for logger "mozbuild.frontend.reader"
[task 2018-09-25T19:26:08.782Z] No build detected, test metadata may be incomplete.
[task 2018-09-25T19:26:08.782Z] now in the try for the objdir path
[task 2018-09-25T19:26:08.782Z] has it found build?: <mozbuild.base.MozbuildObject object at 0x7f4f0e4f8ed0>
[task 2018-09-25T19:26:08.782Z] dirs value is: (u'/builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu',)
[task 2018-09-25T19:26:08.782Z] d is: /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu


So from what I gather, mozbase tests rely on the fact that mozinfo.find_and_update_from_json() no-ops.

With regard to :ahal:, I agree that having harness maintainers handle it is not a good approach. Ideal scenario is to fix behavior of find_and_update_from_json(). 

One complicating factor in the parameter approach is that Python2.7 does not support mixing of keyword arguments with defaults after positional arguments. See

Ideally the signature of the function is not modified, but since Python2.7 is still current for us this may require an approach that may prove brittle - to use a special keyword argument in the positional argument, e.g.:

mozinfo.find_and_update_from_json('path1', 'path2', True)

Where the True boolean maps to an internal raise_exception variable.

Comment 6

9 months ago
Small try run of a proposed fix:

Note the python2/3(mb) tests are now passing.

Proposed patch maintains signature of the original method, but adds ability for caller to toggle if exceptions are desired.


Full try run:

Comment 8

9 months ago
- added codepath prior to main portion to check if caller has passed in an optional raise_exception boolean value.
- if boolean value is detected, function enters new codepath which raises an exception.
- if boolean value is not detected, function continues to behave as it did prior to the patch.

Comment 9

9 months ago
- changed placement of the raise_exception logic to be after the initial objdir path.
- original implementation was missing detection for False cases, changed logic to detect such cases.
- added unit tests to check for scenarios where raise_exception flag is passed in as part of positional argument.

Depends on D6859
I think the value in this bug is in avoiding the frustration expressed in comment 0: A current or future client who calls find_and_update_from_json(path) should be able to expect something bad to happen if path does not exist or is otherwise invalid. 

If using this raise_exception parameter, I think it should be True by default: An exception is raised unless the caller explicitly requests that exceptions are suppressed. But actually, I'd prefer that find_and_update_from_json() always raised an exception on failure, and we rely on callers to try/catch where needed:

except IOError:"I tried to update mozinfo from %s, but that was not found...but that is okay in this case" % path)
Comment on attachment 9011983 [details]
Bug 1305743 - Add optional keyword argument raise_exception to mozinfo.find_and_update_from_json r?gbrown,ahal

Andrew Halberstadt [:ahal] has approved the revision.
Attachment #9011983 - Flags: review+
Attachment #9011982 - Attachment is obsolete: true
Comment on attachment 9011983 [details]
Bug 1305743 - Add optional keyword argument raise_exception to mozinfo.find_and_update_from_json r?gbrown,ahal

Geoff Brown [:gbrown] has approved the revision.
Attachment #9011983 - Flags: review+

Comment 13

9 months ago
Pushed by
Make failure to find mozinfo.json a fatal error r=gbrown,ahal

Comment 14

9 months ago
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1495601
Attachment #9011983 - Attachment description: Bug 1305743 - Make failure to find mozinfo.json a fatal error r?gbrown,ahal → Bug 1305743 - Add optional keyword argument raise_exception to mozinfo.find_and_update_from_json r?gbrown,ahal


9 months ago
Resolution: FIXED → ---

Comment 15

9 months ago
Reopened issue due to issues caused downstream by this change.

New Phabricator diff is up that addresses concerns raised against changing behavior of mozinfo.find_and_update_by_json().


- default to pre-patch behavior, where errors are suppressed. To raise an explicit error, user must pass an optional value 'raise_exception'.
Backed out on request from jgraham for causing issues with mozinfo.json

Push with failures:,x64,opt,python,2,unit,tests,source-test-python-marionette-harness-py2,py2(mnh)&selectedJob=203128422

Failure log:

Backout link:

[task 2018-10-03T15:59:14.146Z]  3:33.14 TEST-START | testing/marionette/harness/marionette_harness/tests/harness_unit/[enabled-True]
[task 2018-10-03T15:59:14.146Z]  3:33.14 TEST-UNEXPECTED-FAIL | testing/marionette/harness/marionette_harness/tests/harness_unit/[enabled-True] | EnvironmentError: mozinfo.mozinfo: could not find any mozinfo.json. (line 267)
[task 2018-10-03T15:59:14.146Z]  3:33.14 mock_runner = <marionette_harness.runtests.MarionetteTestRunner object at 0x7f831881f050>
[task 2018-10-03T15:59:14.146Z]  3:33.14 manifest_with_tests = <ManifestFixture enabled>
[task 2018-10-03T15:59:14.146Z]  3:33.14 monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7f831881f910>
[task 2018-10-03T15:59:14.146Z]  3:33.14 test_files_exist = True
[task 2018-10-03T15:59:14.146Z]  3:33.14 
[task 2018-10-03T15:59:14.146Z]  3:33.14     @pytest.mark.parametrize("test_files_exist", [True, False])
[task 2018-10-03T15:59:14.146Z]  3:33.14     def test_add_test_manifest(mock_runner, manifest_with_tests, monkeypatch, test_files_exist):
[task 2018-10-03T15:59:14.146Z]  3:33.14         monkeypatch.setattr('marionette_harness.runner.base.TestManifest',
[task 2018-10-03T15:59:14.146Z]  3:33.14                             manifest_with_tests.manifest_class)
[task 2018-10-03T15:59:14.146Z]  3:33.14         mock_runner.marionette = mock_runner.driverclass()
[task 2018-10-03T15:59:14.146Z]  3:33.14         with patch('marionette_harness.runner.base.os.path.exists', return_value=test_files_exist):
[task 2018-10-03T15:59:14.146Z]  3:33.14             if test_files_exist or manifest_with_tests.n_enabled == 0:
[task 2018-10-03T15:59:14.146Z]  3:33.14 >               mock_runner.add_test(manifest_with_tests.filepath)
[task 2018-10-03T15:59:14.146Z]  3:33.14 
[task 2018-10-03T15:59:14.146Z]  3:33.14 testing/marionette/harness/marionette_harness/tests/harness_unit/
[task 2018-10-03T15:59:14.146Z]  3:33.14 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[task 2018-10-03T15:59:14.146Z]  3:33.14 testing/marionette/harness/marionette_harness/runner/ in add_test
[task 2018-10-03T15:59:14.146Z]  3:33.14     json_path = update_mozinfo(filepath)
[task 2018-10-03T15:59:14.147Z]  3:33.14 testing/marionette/harness/marionette_harness/runner/ in update_mozinfo
[task 2018-10-03T15:59:14.147Z]  3:33.14     return mozinfo.find_and_update_from_json(*dirs)
[task 2018-10-03T15:59:14.147Z]  3:33.14 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[task 2018-10-03T15:59:14.147Z]  3:33.14 
[task 2018-10-03T15:59:14.147Z]  3:33.14 dirs = ('/path/to/fake/manifest.ini', '/path/to', '/path/to/fake', '/', '/path')
[task 2018-10-03T15:59:14.147Z]  3:33.14 kwargs = {}, MozbuildObject = <class 'mozbuild.base.MozbuildObject'>
[task 2018-10-03T15:59:14.147Z]  3:33.14 BuildEnvironmentNotFoundException = <class 'mozbuild.base.BuildEnvironmentNotFoundException'>
[task 2018-10-03T15:59:14.147Z]  3:33.14 MozconfigFindException = <class 'mozbuild.mozconfig.MozconfigFindException'>
[task 2018-10-03T15:59:14.148Z]  3:33.14 build = <mozbuild.base.MozbuildObject object at 0x7f8318832310>
[task 2018-10-03T15:59:14.148Z]  3:33.14 json_path = '/path/mozinfo.json', raise_exception = True, d = '/path'
[task 2018-10-03T15:59:14.148Z]  3:33.14 
[task 2018-10-03T15:59:14.148Z]  3:33.14     def find_and_update_from_json(*dirs, **kwargs):
[task 2018-10-03T15:59:14.148Z]  3:33.14         """Find a mozinfo.json file, load it, and update global symbol table.
[task 2018-10-03T15:59:14.148Z]  3:33.14 
[task 2018-10-03T15:59:14.149Z]  3:33.14         This method will first check the relevant objdir directory for the
[task 2018-10-03T15:59:14.149Z]  3:33.14         necessary mozinfo.json file, if the current script is being run from a
[task 2018-10-03T15:59:14.149Z]  3:33.14         Mozilla objdir.
[task 2018-10-03T15:59:14.149Z]  3:33.14 
[task 2018-10-03T15:59:14.150Z]  3:33.14         If the objdir directory did not supply the necessary data, this method
[task 2018-10-03T15:59:14.150Z]  3:33.14         will then look for the required mozinfo.json file from the provided
[task 2018-10-03T15:59:14.150Z]  3:33.14         tuple of directories.
[task 2018-10-03T15:59:14.150Z]  3:33.14 
[task 2018-10-03T15:59:14.151Z]  3:33.14         If file is found, the global symbols table is updated via a helper method.
[task 2018-10-03T15:59:14.151Z]  3:33.14 
[task 2018-10-03T15:59:14.152Z]  3:33.14         If no valid files are found, an exception is raised.
[task 2018-10-03T15:59:14.152Z]  3:33.14 
[task 2018-10-03T15:59:14.152Z]  3:33.14         :param tuple dirs: Directories in which to look for the file.
[task 2018-10-03T15:59:14.153Z]  3:33.14         :param dict kwargs: optional values:
[task 2018-10-03T15:59:14.153Z]  3:33.14                             raise_exception: if this value is provided, the default
[task 2018-10-03T15:59:14.153Z]  3:33.14                                              behavior of raising an exception is
[task 2018-10-03T15:59:14.153Z]  3:33.14                                              overridden.
[task 2018-10-03T15:59:14.154Z]  3:33.14         :returns: EnvironmentError: default behavior.
[task 2018-10-03T15:59:14.155Z]  3:33.14                   None: if exception raising is suppressed.
[task 2018-10-03T15:59:14.155Z]  3:33.14                   json_path: string representation of path.
[task 2018-10-03T15:59:14.155Z]  3:33.14         """
[task 2018-10-03T15:59:14.156Z]  3:33.14         # First, see if we're in an objdir
[task 2018-10-03T15:59:14.156Z]  3:33.14         try:
[task 2018-10-03T15:59:14.156Z]  3:33.14             from mozbuild.base import MozbuildObject, BuildEnvironmentNotFoundException
[task 2018-10-03T15:59:14.156Z]  3:33.14             from mozbuild.mozconfig import MozconfigFindException
[task 2018-10-03T15:59:14.157Z]  3:33.14             build = MozbuildObject.from_environment()
[task 2018-10-03T15:59:14.157Z]  3:33.15             json_path = _os.path.join(build.topobjdir, "mozinfo.json")
[task 2018-10-03T15:59:14.157Z]  3:33.15             if _os.path.isfile(json_path):
[task 2018-10-03T15:59:14.157Z]  3:33.15                 update(json_path)
[task 2018-10-03T15:59:14.157Z]  3:33.15                 return json_path
[task 2018-10-03T15:59:14.158Z]  3:33.15         except ImportError:
[task 2018-10-03T15:59:14.158Z]  3:33.15             pass
[task 2018-10-03T15:59:14.158Z]  3:33.15         except (BuildEnvironmentNotFoundException, MozconfigFindException):
[task 2018-10-03T15:59:14.158Z]  3:33.15             pass
[task 2018-10-03T15:59:14.158Z]  3:33.15 
[task 2018-10-03T15:59:14.158Z]  3:33.15         raise_exception = kwargs.get('raise_exception', True)
[task 2018-10-03T15:59:14.159Z]  3:33.15 
[task 2018-10-03T15:59:14.159Z]  3:33.15         for d in dirs:
[task 2018-10-03T15:59:14.159Z]  3:33.15             d = _os.path.abspath(d)
[task 2018-10-03T15:59:14.159Z]  3:33.15             json_path = _os.path.join(d, "mozinfo.json")
[task 2018-10-03T15:59:14.159Z]  3:33.15             if _os.path.isfile(json_path):
[task 2018-10-03T15:59:14.159Z]  3:33.15                 update(json_path)
[task 2018-10-03T15:59:14.159Z]  3:33.15                 return json_path
[task 2018-10-03T15:59:14.160Z]  3:33.15 
[task 2018-10-03T15:59:14.160Z]  3:33.15         if raise_exception:
[task 2018-10-03T15:59:14.161Z]  3:33.15 >           raise EnvironmentError('{}: could not find any mozinfo.json.'.format(__name__))
[task 2018-10-03T15:59:14.161Z]  3:33.15 E           EnvironmentError: mozinfo.mozinfo: could not find any mozinfo.json.
[task 2018-10-03T15:59:14.161Z]  3:33.15 
[task 2018-10-03T15:59:14.161Z]  3:33.15 testing/mozbase/mozinfo/mozinfo/ EnvironmentError
[task 2018-10-03T15:59:14.161Z]  3:33.15 TEST-INFO took 115ms
[task 2018-10-03T15:59:14.161Z]  3:33.15 TEST-START | testing/marionette/harness/marionette_harness/tests/harness_unit/[enabled-False]
[task 2018-10-03T15:59:14.161Z]  3:33.15 TEST-UNEXPECTED-FAIL | testing/marionette/harness/marionette_harness/tests/harness_unit/[enabled-False] | EnvironmentError: mozinfo.mozinfo: could not find any mozinfo.json. (line 267)
[task 2018-10-03T15:59:14.162Z]  3:33.15 mock_runner = <marionette_harness.runtests.MarionetteTestRunner object at 0x7f831892ec10>
Flags: needinfo?(egao)
Target Milestone: mozilla64 → ---

Comment 18

9 months ago
Pushed by
Make failure to find mozinfo.json a fatal error r=gbrown,ahal a=reland

Comment 19

9 months ago
:jgraham Would it be a viable solution for me to fix the way mozinfo.find_and_update_from_json() is being called with the patch applied? The original bug description states to remove the no-op behavior as this is/was causing other silent failures.

:CosminS Same applies here - would it be an option for me to fix the unit test that was failing in
Flags: needinfo?(james)
Flags: needinfo?(egao)
Flags: needinfo?(csabou)
:egao, from what I spoke with jgraham on IRC we agreed that backing out this bug and bug 1495601 would solve the problem for now until you or somebody else will find a fix for this in a different (non-breaking) way. That bug there 1496141 will no longer fail after the backout.
Flags: needinfo?(csabou)

Comment 21

9 months ago
Backout by
Backed out changeset dcba2a476ccf on request from jgraham for causing issues with mozinfo.json. a=backout
I think the right fix would be to make the behaviour opt-in, since there are many cases where not finding a mozinfo file isn't a problem.
Flags: needinfo?(james)
:egao - What is your plan here? Under the circumstances, I think opt-in would be okay. I could also see closing this as wontfix.
Flags: needinfo?(egao)

Comment 24

7 months ago
(In reply to Geoff Brown [:gbrown] from comment #23)
> :egao - What is your plan here? Under the circumstances, I think opt-in
> would be okay. I could also see closing this as wontfix.

I will take another stab at fixing this without impacting current automation/processes, now that I have some more experience under my belt.
Flags: needinfo?(egao)

Comment 25

7 months ago
Newly attempted solution closely mirrors the last attempt in Phabricator diff D6860.

Preliminary Try run to validate build:

- the preliminary try run is coming back all green.

Handpicked sets of tests to check changes:

- tests do not appear to be affected by the changes to mozinfo.

Python2/3 unit tests:

- python unit tests complete, no failures.

Comment 26

7 months ago
- added optional keyword argument to the find_and_update_from_json() function: raise_exception
- the optional keyword argument accepts a boolean value depending on if exception is desired.

- added a few tests to ensure the expected exceptions are raised only when raise_exception flag is True.

Comment 27

7 months ago
Pushed by
Add optional parameter to make failure to find mozinfo.json a fatal error r=gbrown

Comment 28

7 months ago
Closed: 9 months ago7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.