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

Harvest the "without changes" runs from Taskcluster #865

Merged
merged 2 commits into from
Dec 6, 2018

Conversation

Hexcles
Copy link
Member

@Hexcles Hexcles commented Dec 6, 2018

This change extends the set of valid task names to accept the
"*-results-without-changes" tasks that are being added in
web-platform-tests/wpt#14382 . The change also
adds a "pr_base" label to these special runs, and "pr_head" to the PR
runs with the changes to prepare for the next step: using these runs
to calculate PR regressions (not implemented in this change).

Related to #708 .

This change extends the set of valid task names to accept the
"*-results-without-patch" tasks that are being added in
web-platform-tests/wpt#14382 . The change also
adds a "without_patch" label to these special runs to prepare for the
next step: using these runs to calculate PR regressions (not implemented
in this change).
@Hexcles Hexcles requested a review from lukebjerring December 6, 2018 01:45
@wpt-pr-bot
Copy link

Staging instance deployed by Travis CI!
Running at https://taskcluster-without-patch-dot-wptdashboard-staging.appspot.com

@wpt-pr-bot
Copy link

Staging instance deployed by Travis CI!
Running at http://taskcluster-without-patch.announcer.wptdashboard-staging.appspot.com

shared/util.go Outdated
@@ -33,6 +33,10 @@ const BetaLabel = "beta"
// i.e. run from the master branch.
const MasterLabel = "master"

// WithoutPatchLabel is the label for running just the affected tests on a PR
// but without the patch.
const WithoutPatchLabel = "without_patch"
Copy link
Contributor

Choose a reason for hiding this comment

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

So... I did say that eventually master might stop implying a full run, and this is that day.
I'm wondering if labelling the runs that aren't just the affected tests as full is a better route for our default homepage.

Alternatively (or even in conjunction with full) we can pick a more succinct name for this label. How about we go the science route, and label them as control? Or baseline? normal? prior?
... I think baseline might be best, since we use that term with the same semantic meaning in other contexts.

Copy link
Contributor

Choose a reason for hiding this comment

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

@foolip suggested base (without_patch) and head (with patch) offline. They work IMHO, WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Then I threw pr_base and pr_head into the mix. Either is fine, only possible misreading is that head could mean HEAD, i.e. master.

Copy link
Member Author

Choose a reason for hiding this comment

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

Back to your first comment, Luke. I understand master eventually won't be full runs in the future, but this PR doesn't change the current status that master runs are always full runs, right?

I like pr_base and pr_head, for the overuse of the term "head" itself.

We settled down on the names of Taskcluster tasks (already landed in
WPT) and labels on wpt.fyi.
@Hexcles
Copy link
Member Author

Hexcles commented Dec 6, 2018

@lukebjerring PTAL again

@Hexcles Hexcles changed the title Harvest the "without patch" runs from Taskcluster Harvest the "without changes" runs from Taskcluster Dec 6, 2018
@Hexcles Hexcles merged commit 4d96621 into master Dec 6, 2018
@Hexcles Hexcles deleted the taskcluster-without-patch branch December 6, 2018 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants