1569838 - ./mach mochitest --subsuite webgl2-core should not require git2hg integration

Status: NEW

(Testing :: General, defect, P3)

Description

[Kelsey Gilbert [:jgilbert]]

mozs-MacBook-Pro:gecko1 jgilbert$ ./mach mochitest --subsuite webgl2-core
git: 'cinnabar' is not a git command. See 'git --help'.
Error running mach:

['mochitest', '--subsuite', 'webgl2-core']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

CalledProcessError: Command '['git', 'cinnabar', 'git2hg', 'eb05e0ba5020a806912aca53557dfd77629ffd56']' returned non-zero exit status 1

File "/Users/jgilbert/dev/mozilla/gecko1/testing/mochitest/mach_commands.py", line 362, in run_mochitest_general
tests = mochitest.resolve_tests(test_paths, test_objects, cwd=self._mach_context.cwd)
File "/Users/jgilbert/dev/mozilla/gecko1/testing/mochitest/mach_commands.py", line 105, in resolve_tests
tests = list(resolver.resolve_tests(paths=test_paths, cwd=cwd))
File "/Users/jgilbert/dev/mozilla/gecko1/testing/mozbase/moztest/moztest/resolve.py", line 588, in resolve_tests
for test in result:
File "/Users/jgilbert/dev/mozilla/gecko1/testing/mozbase/moztest/moztest/resolve.py", line 429, in resolve_tests
self.add_wpt_manifest_data()
File "/Users/jgilbert/dev/mozilla/gecko1/testing/mozbase/moztest/moztest/resolve.py", line 482, in add_wpt_manifest_data
logger=logger)
File "/Users/jgilbert/dev/mozilla/gecko1/testing/web-platform/manifestupdate.py", line 113, in run
force=force_download)
File "/Users/jgilbert/dev/mozilla/gecko1/testing/web-platform/manifestdownload.py", line 171, in download_from_taskcluster
force)
File "/Users/jgilbert/dev/mozilla/gecko1/testing/web-platform/manifestdownload.py", line 126, in download_manifest
url = url_func(logger, commits)
File "/Users/jgilbert/dev/mozilla/gecko1/testing/web-platform/manifestdownload.py", line 79, in taskcluster_url
for revision in commits:
File "/Users/jgilbert/dev/mozilla/gecko1/testing/web-platform/manifestdownload.py", line 37, in git_commits
yield git("cinnabar", "git2hg", item).strip()
File "/Users/jgilbert/dev/mozilla/gecko1/testing/web-platform/vcs.py", line 38, in git
return subprocess.check_output(full_cmd, cwd=repo_root)
File "/usr/local/Cellar/python@2/2.7.16/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 223, in check_output
raise CalledProcessError(retcode, cmd, output=output)
mozs-MacBook-Pro:gecko1 jgilbert$

There's no reason to require git2hg for this usecase. Because I have a large number of development machines, I often develop on systems without git cinnabar (because it's a DVCS, so it shouldn't be needed), and in particular running a non-WPT subsuite should not incur WPT testrunning requirements.

Comment 1

[Geoff Brown [:gbrown]]

Was mercurial available? I would have expected

https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/testing/web-platform/manifestdownload.py#40

to have selected hg if it was available. (I recognize there are other issues, like why is web-platform involved and why is vcs required at all, but I'd like to check on this detail too.)

Comment 2

[Kelsey Gilbert [:jgilbert]]

Sorry, this is for a git repo where its origin is another centralized machine that handles git cinnabar. In this case, I have git installed, but not hg or git-cinnabar.

Comment 3

[James Graham [:jgraham]]

So I think there are two things happening here:

• Running on git without cinnabar isn't a supported configuration. If you want to clone git repos around you need to ensure that you adjust the refspec to include refs/cinnbar/* and refs/notes/cinnabar and install cinnabar on all the machines.
• The mochtest harness calls the test resolver in a way that demands all tests are resolved. That seems unnecessary, and I wonder why we don't pass at least flavor="mochitest".

Comment 4

[Kelsey Gilbert [:jgilbert]]

I've found maintaining a dozen copies of git cinnabar, arc/phlay and friends onerous, so it would be really cool if we had less tight coupling between the code and the repo.
Looser coupling is more robust for many reasons regardless, but this is the first time I've ever been stymied by my unorthodox (but not exotic) DVCS setup.

What's even going on with WPT that it wants to know anything from the repo details? That surprises me, and certainly seems unnecessary.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:gbrown, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[James Graham [:jgraham]]

(In reply to Jeff Gilbert [:jgilbert] from comment #4)

I've found maintaining a dozen copies of git cinnabar, arc/phlay and friends onerous, so it would be really cool if we had less tight coupling between the code and the repo.
Looser coupling is more robust for many reasons regardless, but this is the first time I've ever been stymied by my unorthodox (but not exotic) DVCS setup.

AIUI hg and git+cinnabar are the only supported configurations. I understand the desire to just git clone and be done with it, but that's pushing the cost of handling a greater range of environments onto other people.

What's even going on with WPT that it wants to know anything from the repo details? That surprises me, and certainly seems unnecessary.

It needs to know the last revision(s) to touch wpt tests in order to download a test manifest. The previous system of keeping the manifest in tree broke every system that didn't like a ~20Mb JSON blob — which was almost all of them — and regenerating it from the source is prohibitively slow.

Comment 7

[Kelsey Gilbert [:jgilbert]]

This is a trade-off of strong vs weak coupling, and I think making things more modular here has benefits beyond just what I'm doing today.

Why couldn't we just put the uri for the test manifest in the tree?

I would at least like to not have this problem for non-wpt tests, and that seems like it should be simple enough.

Comment 8

[James Graham [:jgraham]]

I agree that there is a real bug here in the mochitest harness.

For wpt the manifest changes when tests are updated, so the url changes depending on commits in the tree. You could probably make a patch to have it fall back to some fixed url in the case where cinnabar isn't found, but since that's not a supported configuration in general I wouldn't want to add any significant complexity to handle that use case, or give any expectation that the code will continue working without cinnabar going forward. Nevertheless if you make a patch and it's straightforward I'll happily review it.