Skip to content

/ servo

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

test-tidy command now ignores files in subdirectories of ignored dirs #12313

Merged
merged 2 commits into from Jul 8, 2016
Merged

test-tidy command now ignores files in subdirectories of ignored dirs #12313

merged 2 commits into from Jul 8, 2016

Conversation

@cynicaldevil
Copy link
Contributor

cynicaldevil commented Jul 7, 2016 •
edited

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #12225 .

I did not add a test for this, instead I added a directory with an empty file inside the ignored directory, and checked whether this file was being ignored or not.

This change is Reviewable
cynicaldevil added 2 commits Jul 7, 2016
@cynicaldevil
tidy command now ignores files in subdirectories of ignored directories
be98fc4
@cynicaldevil
Added a subdirectory with a file to test whether tidy command ignores…
Loading status checks…
6c6bfdb 
… files in subdirectories
@highfive
Copy link

highfive commented Jul 7, 2016

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @SimonSapin (or someone else) soon.
@highfive
Copy link

highfive commented Jul 7, 2016

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/tidy/servo_tidy/tidy.py, python/tidy/servo_tidy_tests/test_ignored/whee/foo/bar.rs
@KiChjang
Copy link
Member

KiChjang commented Jul 7, 2016

Could you put invalid syntax in the bar.rs file to ensure that test-tidy did ignore that directory/file?
@wafflespeanut
Copy link
Member

wafflespeanut commented Jul 7, 2016

No, that shouldn't be a problem. The existing unittest checks for this.
@KiChjang
Copy link
Member

KiChjang commented Jul 7, 2016

Ah ok, forget what I mentioned.
@wafflespeanut
Copy link
Member

wafflespeanut commented Jul 7, 2016

Looks good to me! Thanks for doing this :)

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

📌 Commit 6c6bfdb has been approved by Wafflespeanut
@highfive highfive assigned wafflespeanut and unassigned SimonSapin Jul 7, 2016
@cynicaldevil
Copy link
Contributor Author

cynicaldevil commented Jul 7, 2016

I'm curious...what would happen if someone pushed new commits in the time between the PR was approved and when it gets merged by the bot...since users aren't notified when new commits are pushed to the branch. Are there any checks to prevent this from happening?
@KiChjang
Copy link
Member

KiChjang commented Jul 7, 2016

Homu would reject the previously accepted PR and a reviewer would have to r+ again in order to instruct homu to rebuild the PR and accept it.
@cynicaldevil
Copy link
Contributor Author

cynicaldevil commented Jul 7, 2016

Ah, I see.
bors-servo added a commit that referenced this pull request Jul 7, 2016
@bors-servo
Auto merge of #12313 - cynicaldevil:tidy-test, r=Wafflespeanut
5c50d90 
test-tidy command now ignores files in subdirectories of ignored dirs

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12225 .

I did not add a test for this, instead I added a directory with an empty file inside the ignored directory, and checked whether this file was being ignored or not.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12313)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

⌛ Testing commit 6c6bfdb with merge 5c50d90...
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

💔 Test failed - mac-rel-css
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 7, 2016

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jul 8, 2016

⌛ Testing commit 6c6bfdb with merge 9c00331...
bors-servo added a commit that referenced this pull request Jul 8, 2016
@bors-servo
Auto merge of #12313 - cynicaldevil:tidy-test, r=Wafflespeanut
Loading status checks…
9c00331 
test-tidy command now ignores files in subdirectories of ignored dirs

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12225 .

I did not add a test for this, instead I added a directory with an empty file inside the ignored directory, and checked whether this file was being ignored or not.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12313)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 8, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows
@bors-servo bors-servo merged commit 6c6bfdb into servo:master Jul 8, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@cynicaldevil cynicaldevil deleted the cynicaldevil:tidy-test branch Jul 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@wafflespeanut wafflespeanut

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

./mach test-tidy doesn't ignore files underneath ignored directories
7 participants
@cynicaldevil @highfive @KiChjang @wafflespeanut @bors-servo @larsbergstrom @SimonSapin
You can’t perform that action at this time.