Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Chrome nightly (Chromium trunk) to wpt #24702

Merged
merged 5 commits into from Jul 24, 2020
Merged

Add Chrome nightly (Chromium trunk) to wpt #24702

merged 5 commits into from Jul 24, 2020

Conversation

Hexcles
Copy link
Member

@Hexcles Hexcles commented Jul 22, 2020

Chrome does not really have an official "nightly" channel. We are using it to refer to the Chromium trunk (tip of the tree) build (available via https://download-chromium.appspot.com/).

This PR enables us to

  • wpt install [--download-only] [--channel nightly] chrome browser and wpt install [--channel nightly] chrome webdriver
  • wpt run --install-browser --channel nightly chrome (experimental web platform features will be enabled, same as dev or canary)

Note that this changes the default / latest channel for Chrome to nightly when no channel is supplied, which won't break the CI because we always specify the channel.

It might be easier to review the commits individually.

@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24702 July 22, 2020 22:53 Pending
@Hexcles Hexcles force-pushed the chromium-nightly branch 2 times, most recently from 0f576e0 to cf8d66f Compare July 23, 2020 14:38
@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24702 July 23, 2020 14:40 Pending
Add a test for installing Chrome nightly.

In addition, create a safer version of shutil.rmtree based on existing
implementation in browser.py and reuse it in more places in place of
shutil.rmtree.
Copy link
Contributor

@LukeZielinski LukeZielinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm%nits

def _get_dest(self, dest, channel):
if dest is None:
# os.getcwd() doesn't include the venv path
dest = os.path.join(os.getcwd(), "_venv")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More for my own info - isn't the _venv dir suffixed with the py version these days? Curious why it doesn't care about that here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it is, and AFAICT, dest is never None when executed from wpt run or wpt install, but I'll leave it alone for now until I do some further investigation for a cleanup/refactoring.

tools/wpt/browser.py Outdated Show resolved Hide resolved
tools/wpt/tests/test_wpt.py Outdated Show resolved Hide resolved
tools/wpt/browser.py Outdated Show resolved Hide resolved
@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24702 July 24, 2020 14:15 Pending
tools/wpt/browser.py Outdated Show resolved Hide resolved
tools/wpt/tests/test_wpt.py Outdated Show resolved Hide resolved
tools/wpt/browser.py Outdated Show resolved Hide resolved
@stephenmcgruer
Copy link
Contributor

Drive-by: what are these runs treated as when they reach wpt.fyi? Will they be selectable separately from 'experimental' (which is what dev is currently tagged as). Will they be mixed in with dev?

@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24702 July 24, 2020 14:57 Pending
@Hexcles
Copy link
Member Author

Hexcles commented Jul 24, 2020

I definitely don't plan to mix it with dev for various reasons (it's not the same channel, or even the same "product" in a strict sense).

For now, these runs will be more like a trial for enabling MojoJS until the next branch point (when mojojs.zip will be available for Dev) and will be given either special labels or a different product name ("chromium" instead of "chrome") on wpt.fyi to hide it from the homepage by default. They'll be scheduled less frequently (maybe daily. We may likely later find them more useful than just experimenting with MojoJS, and then we can revisit this (maybe even replace dev with Chromium ToT on wpt.fyi) as a separate discussion.

@stephenmcgruer
Copy link
Contributor

Right, I'm glad that that is the intent, but I also want to make sure we've thought through whether that is what will happen.

For example, looking at wpt.fyi APIs, these are the tags for Chrome Dev and Firefox Nightly runs from the Taskcluster master push:

["experimental","firefox","master","nightly","taskcluster","user:someone"]
["chrome","dev","experimental","master","taskcluster","user:someone"]

Now, if we assume that Chromium ToT would look like (because the results-processor maps nightly to experimental):

["chrome","nightly","experimental","master","taskcluster","user:someone"]

Then it's important that the wpt.fyi front-end knows how to distinguish between Chrome Dev and Nightly, because if it just uses 'experimental' we're going to mix results.

Does that make sense? Apologies if this has already been thought through.

@Hexcles
Copy link
Member Author

Hexcles commented Jul 24, 2020

Oh sorry I forgot to mention that a change is needed in wpt.fyi, too, which is also partly why this PR does not actually schedule anything. But thanks for double checking!

@Hexcles Hexcles merged commit d745272 into master Jul 24, 2020
@Hexcles Hexcles deleted the chromium-nightly branch July 24, 2020 17:05
Hexcles added a commit that referenced this pull request Jul 27, 2020
This was accidentally regressed in #24702 due to lack of comments or
tests, both of which are also added in this change.

Fixes #24753.
Hexcles added a commit that referenced this pull request Jul 27, 2020
This was accidentally regressed in #24702 due to lack of comments or
tests, both of which are also added in this change.

Fixes #24753.
Hexcles added a commit to web-platform-tests/wpt.fyi that referenced this pull request Jul 28, 2020
This change refactors the logic for adding release channel labels based
on browser_channel so that the mapping is browser-dependent, with two
changes in behaviours:

1. Treat Chrome Nightly differently from Firefox Nightly. Chrome Nightly
   is in fact Chromium ToT (recently added in
   web-platform-tests/wpt#24702), and we don't
   want to add the experimental label to it which would mix it with the
   official Chrome Dev.
2. Add a "nightly" label to Chrome/Edge Canary (we only have Edge Canary
   runs as of now) to allow easy comparison between e.g. Chrome Nightly
   (Chromium ToT) and Edge Canary.
Hexcles added a commit to web-platform-tests/wpt.fyi that referenced this pull request Jul 28, 2020
This change refactors the logic for adding release channel labels based
on browser_channel so that the mapping is browser-dependent, with two
changes in behaviours:

1. Treat Chrome Nightly differently from Firefox Nightly. Chrome Nightly
   is in fact Chromium ToT (recently added in
   web-platform-tests/wpt#24702), and we don't
   want to add the experimental label to it which would mix it with the
   official Chrome Dev.
2. Add a "nightly" label to Chrome/Edge Canary (we only have Edge Canary
   runs as of now) to allow easy comparison between e.g. Chrome Nightly
   (Chromium ToT) and Edge Canary.
Hexcles added a commit to web-platform-tests/wpt.fyi that referenced this pull request Aug 4, 2020
This change refactors the logic for adding release channel labels based
on browser_channel so that the mapping is browser-dependent, with two
changes in behaviours:

1. Treat Chrome Nightly differently from Firefox Nightly. Chrome Nightly
   is in fact Chromium ToT (recently added in
   web-platform-tests/wpt#24702), and we don't
   want to add the experimental label to it which would mix it with the
   official Chrome Dev.
2. Add a "nightly" label to Chrome/Edge Canary (we only have Edge Canary
   runs as of now) to allow easy comparison between e.g. Chrome Nightly
   (Chromium ToT) and Edge Canary.

Drive-by: add logging for the default fallback branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants