Skip to content

Allow running long tests with testOnly #3227

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 3, 2017
Merged

Allow running long tests with testOnly #3227

merged 4 commits into from
Oct 3, 2017

Conversation

nicolasstucki
Copy link
Contributor

Improves #3194

@nicolasstucki nicolasstucki mentioned this pull request Oct 1, 2017
Merged
@nicolasstucki nicolasstucki requested a review from smarter October 1, 2017 18:06
@allanrenucci
Copy link
Contributor

What about a solution based on junit categories?

smarter
smarter previously approved these changes Oct 1, 2017
View reviewed changes
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM, but Allan's suggestion seems worth exploring too

@nicolasstucki nicolasstucki dismissed smarter’s stale review October 2, 2017 09:35

Modified code to use JUnit categories

allanrenucci
allanrenucci reviewed Oct 2, 2017
View reviewed changes
project/Build.scala Outdated
@@ -465,13 +465,13 @@ object Build {

test in Test := {
// Exclude legacy tests by default
(testOnly in Test).toTask(" -- --exclude-categories=java.lang.Exception").value
(testOnly in Test).toTask(" -- --exclude-categories=java.lang.Exception,dotty.tools.dotc.SlowTests").value
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that Drone will not run them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it won't. I will have to find a workaround

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

allanrenucci
allanrenucci approved these changes Oct 3, 2017
View reviewed changes
Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

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

LGTM

project/Build.scala Outdated
},

testAll in Test := {
// Exclude legacy tests by default
(testOnly in Test).toTask(" -- --exclude-categories=java.lang.Exception").value
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're at it, could you create a meaningful category for legacy tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will

@nicolasstucki
Copy link
Contributor Author

@allanrenucci could you review the las commit

@allanrenucci
Copy link
Contributor

Can we put all the categories in one file. Maybe under compiler/test/dotty/TestCategories. Also I think you should be able to do @Category(classOf[SlowTests]) instead of @Category(Array(classOf[SlowTests])) when a test belongs to only one category. Otherwise LGTM

@nicolasstucki
Copy link
Contributor Author

It does not work with @Category(classOf[SlowTests]), probably implementation details of the JUnit plugin.

@nicolasstucki nicolasstucki merged commit 96b0ae4 into scala:master Oct 3, 2017
@allanrenucci allanrenucci deleted the improve-#3194 branch December 14, 2017 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allanrenucci allanrenucci allanrenucci approved these changes

@smarter smarter smarter left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nicolasstucki @allanrenucci @smarter