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


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

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


        Auto merge of #9118 - dsprenkels:test-tidy-faster, r=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...


        Auto merge of #9118 - dsprenkels:test-tidy-faster, r=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-08T17:04:14Z

💔 Test failed - linux-rel

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

@bors-servo retry

#8769

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

⌛ Testing commit a9b8d47 with merge 7422683...


        Auto merge of #9118 - dsprenkels:test-tidy-faster, r=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)

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

#8769, #9205
Blame @wafflespeanut for jinxing it

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 label Jan 1, 2016

wafflespeanut self-assigned this Jan 2, 2016

wafflespeanut added S-needs-decision and removed S-awaiting-review labels Jan 4, 2016

wafflespeanut added S-needs-code-changes and removed S-needs-decision labels Jan 4, 2016

highfive added S-awaiting-review and removed S-needs-code-changes 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 and removed S-awaiting-review labels Jan 6, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Jan 6, 2016

wafflespeanut added S-needs-squash and removed S-awaiting-review labels Jan 7, 2016

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

Loading status checks…

a9b8d47

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

highfive added the S-awaiting-review label Jan 7, 2016

highfive removed S-awaiting-review S-needs-squash labels Jan 7, 2016

nox removed the S-tests-failed label Jan 8, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 8, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Jan 8, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 8, 2016

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

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

dsprenkels deleted the dsprenkels:test-tidy-faster branch Jan 8, 2016

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 label Jan 27, 2016

servo / servo

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

servo / servo

Join GitHub today

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