Add 'test-tidy-faster' command to ./mach #9118

dsprenkels · 2016-01-01T23:51:07Z

For issue #9088, this adds a --changes option to ./mach test-tidy. If this option is set, tidy.py will only check files that have been modified since FETCH_HEAD.

Mention: @wafflespeanut

nox · 2016-01-02T12:51:46Z

I never have a FETCH_HEAD in my repository.

dsprenkels · 2016-01-02T13:06:37Z

@nox, that's inconvenient. Shall I instead use

git log -n1 --author="bors-servo" --format="%H"

to find the last merge?

wafflespeanut · 2016-01-04T17:00:54Z

-S-awaiting-review +S-needs-decision

Reviewed 1 of 3 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

tests/wpt/web-platform-tests/tools/lint/lint.py, line 28 [r2] (raw file):
No! (as @jgraham said, and I quote), we shouldn't have servo-specific code in the WPT dir, because it's a standard used by other browsers too, and not only us.

So, this probably requires more work than I'd previously anticipated. I'll come up with a plan soon :)

Comments from the review on Reviewable.io

dsprenkels · 2016-01-04T18:01:22Z

We could stick to the original plan:

...we could have a mach command test-tidy-faster which ignores the check_wpt_lint_errors function in tidy.

Then we would not have to put any code in the WPT dir, but it would not be a very elegant (and probably just a temporary) solution.

wafflespeanut · 2016-01-04T18:25:27Z

@dsprenkels Agreed, let's have test-tidy-faster for now. I'll see what I can do for the other case :)

dsprenkels · 2016-01-04T23:14:43Z

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions.

python/servo/testing_commands.py, line 262 [r3] (raw file):
I implemented this as a separate command. However, it may be a good idea to add this to test-tidy as an option (-f/--faster). We could otherwise add --skip-example options to this command, which each skip particular test functions (when doing this, we could perhaps alias -f/--faster to --skip-wpt-lint).

python/tidy.py, line 567 [r3] (raw file):
At the moment I still only check the files that are changed. This makes the command run in two seconds (on my machine). Shall I keep this?

python/tidy.py, line 595 [r3] (raw file):
Should I give this line some color?

Comments from the review on Reviewable.io

wafflespeanut · 2016-01-06T17:40:32Z

-S-awaiting-review +S-needs-code-changes

Reviewed 2 of 3 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

python/servo/testing_commands.py, line 262 [r3] (raw file):
Having -f / --faster flag is fine, but we don't wanna skip anything else (for now).

python/tidy.py, line 567 [r3] (raw file):
This would be helpful (pinging @jdm if he's okay with this)...

python/tidy.py, line 595 [r3] (raw file):
cool with me...

Comments from the review on Reviewable.io

jdm · 2016-01-06T18:17:53Z

Review status: all files reviewed at latest revision, 3 unresolved discussions.

python/tidy.py, line 567 [r3] (raw file):
Looks good to me, as long as we ensure that the version that runs on travis doesn't take the fast path.

Comments from the review on Reviewable.io

wafflespeanut · 2016-01-07T14:36:02Z

-S-awaiting-review +S-needs-squash
Looks good! Squash?

Reviewed 2 of 2 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

which will - only check files changed since the last merge by bors - and skip the wpt-lint

wafflespeanut · 2016-01-07T15:38:34Z

@bors-servo r+

Thanks! :)

bors-servo · 2016-01-07T15:38:35Z

📌 Commit a9b8d47 has been approved by Wafflespeanut

@wafflespeanut

Add 'test-tidy-faster' command to ./mach For issue #9088, this adds a `--changes` option to `./mach test-tidy`. If this option is set, `tidy.py` will only check files that have been modified since `FETCH_HEAD`. Mention: @wafflespeanut  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9118)

bors-servo · 2016-01-08T12:30:11Z

⌛ Testing commit a9b8d47 with merge e60f88e...

bors-servo · 2016-01-08T12:42:34Z

⛄ The build was interrupted to prioritize another pull request.

bors-servo · 2016-01-08T13:16:53Z

⚡ Previous build results for gonk are reusable. Rebuilding only android, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt...

bors-servo · 2016-01-08T13:33:49Z

⛄ The build was interrupted to prioritize another pull request.

bors-servo · 2016-01-08T16:29:43Z

⌛ Testing commit a9b8d47 with merge e0f2d40...

@wafflespeanut

Add 'test-tidy-faster' command to ./mach For issue #9088, this adds a `--changes` option to `./mach test-tidy`. If this option is set, `tidy.py` will only check files that have been modified since `FETCH_HEAD`. Mention: @wafflespeanut  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9118)

bors-servo · 2016-01-08T17:04:14Z

💔 Test failed - linux-rel

KiChjang · 2016-01-08T17:19:56Z

@bors-servo retry

Intermittent timeout in /_mozilla/css/iframe/hide_layers2.html (Constellation panic) #8769

bors-servo · 2016-01-08T17:43:00Z

⌛ Testing commit a9b8d47 with merge 7422683...

@wafflespeanut

Add 'test-tidy-faster' command to ./mach For issue #9088, this adds a `--changes` option to `./mach test-tidy`. If this option is set, `tidy.py` will only check files that have been modified since `FETCH_HEAD`. Mention: @wafflespeanut  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9118)

wafflespeanut · 2016-01-08T18:10:02Z

WRATH OF THE BOTS!!!

bors-servo · 2016-01-08T18:17:20Z

💔 Test failed - linux-rel

KiChjang · 2016-01-08T18:20:21Z

@bors-servo retry

bors-servo · 2016-01-08T18:51:34Z

⚡ Previous build results for android, gonk, linux-dev, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only linux-rel...

bors-servo · 2016-01-08T19:25:37Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

wafflespeanut · 2016-01-09T00:49:59Z

Whoo! Finally! :)

dsprenkels · 2016-01-09T00:58:07Z

:D

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 1, 2016

wafflespeanut self-assigned this Jan 2, 2016

wafflespeanut added S-needs-decision A decision is required and has not been made. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 4, 2016

wafflespeanut added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-needs-decision A decision is required and has not been made. labels Jan 4, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jan 4, 2016

dsprenkels changed the title ~~Add '--changes' option to ./mach test-tidy~~ Add 'test-tidy-faster' command to ./mach Jan 4, 2016

wafflespeanut added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 6, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jan 6, 2016

wafflespeanut added S-needs-squash Some (or all) of the commits in the PR should be combined. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 7, 2016

add a --faster option to ./mach test-tidy

a9b8d47

which will - only check files changed since the last merge by bors - and skip the wpt-lint

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 7, 2016

highfive removed S-awaiting-review There is new code that needs to be reviewed. S-needs-squash Some (or all) of the commits in the PR should be combined. labels Jan 7, 2016

nox removed the S-tests-failed The changes caused existing tests to fail. label Jan 8, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 8, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 8, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 8, 2016

bors-servo merged commit a9b8d47 into servo:master Jan 8, 2016

dsprenkels deleted the test-tidy-faster branch January 8, 2016 20:48

jdm mentioned this pull request Jan 11, 2016

./mach test returns error #9253

Closed

wafflespeanut mentioned this pull request Jan 13, 2016

./mach test-tidy-faster #9088

Closed

SimonSapin removed the S-tests-failed The changes caused existing tests to fail. label Jan 27, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'test-tidy-faster' command to ./mach #9118

Add 'test-tidy-faster' command to ./mach #9118

dsprenkels commented Jan 1, 2016

nox commented Jan 2, 2016

dsprenkels commented Jan 2, 2016

wafflespeanut commented Jan 4, 2016

dsprenkels commented Jan 4, 2016

wafflespeanut commented Jan 4, 2016

dsprenkels commented Jan 4, 2016

wafflespeanut commented Jan 6, 2016

jdm commented Jan 6, 2016

wafflespeanut commented Jan 7, 2016

wafflespeanut commented Jan 7, 2016

bors-servo commented Jan 7, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

KiChjang commented Jan 8, 2016

bors-servo commented Jan 8, 2016

wafflespeanut commented Jan 8, 2016

bors-servo commented Jan 8, 2016

KiChjang commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

wafflespeanut commented Jan 9, 2016

dsprenkels commented Jan 9, 2016

Add 'test-tidy-faster' command to ./mach #9118

Add 'test-tidy-faster' command to ./mach #9118

Conversation

dsprenkels commented Jan 1, 2016

nox commented Jan 2, 2016

dsprenkels commented Jan 2, 2016

wafflespeanut commented Jan 4, 2016

dsprenkels commented Jan 4, 2016

wafflespeanut commented Jan 4, 2016

dsprenkels commented Jan 4, 2016

wafflespeanut commented Jan 6, 2016

jdm commented Jan 6, 2016

wafflespeanut commented Jan 7, 2016

wafflespeanut commented Jan 7, 2016

bors-servo commented Jan 7, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

KiChjang commented Jan 8, 2016

bors-servo commented Jan 8, 2016

wafflespeanut commented Jan 8, 2016

bors-servo commented Jan 8, 2016

KiChjang commented Jan 8, 2016

bors-servo commented Jan 8, 2016

bors-servo commented Jan 8, 2016

wafflespeanut commented Jan 9, 2016

dsprenkels commented Jan 9, 2016