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

IncludeExcludeCategory should accept many categories + configurable #461

Merged
merged 0 commits into from
Aug 7, 2012

Conversation

Tibor17
Copy link
Contributor

@Tibor17 Tibor17 commented Jul 14, 2012

This pul request was created after I resolved conflicts with KentBeck:master.
This is the same #354 + #336 .
We discussed this feature in #142 and continued then in #354.

@@ -16,13 +11,60 @@
import org.junit.runners.model.InitializationError;
import org.junit.runners.model.RunnerBuilder;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.*;
Copy link
Member

Choose a reason for hiding this comment

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

JUnit style is never to import *.

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jul 17, 2012

ok, i will fix this.

@Tibor17 Tibor17 mentioned this pull request Jul 18, 2012
@Tibor17
Copy link
Contributor Author

Tibor17 commented Jul 19, 2012

@dsaff
I committed what you said was need:

  • improved imports;
  • links to a particularly helpful set of testcases in class Categories

Is it fine?

* -Dorg.junit.categories.excluded=com/Category3.java,com/Category4.java
* </pre></blockquote>
* <p>
* Thus a class 'com.Category1' has file name 'com/Category1.java'.
Copy link
Member

Choose a reason for hiding this comment

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

Remind me again why you're suggesting using java file names?

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jul 19, 2012

because the
maven-surefire-plugin
does use the same format.

So i wanted to be consistent with maven.

Example of test class and properties in pom.xml:

groupId: maven-surefire-plugin
configuration: test: org/junit/tests/AllTests.java
properties: org.junit.categories.excluded: org/junit/SomeOtherTest.java

But we may change it if necessary.

@dsaff
Copy link
Member

dsaff commented Jul 20, 2012

It looks like surefire already supports categories, and using class names,
not filenames:

http://maven.apache.org/plugins/maven-surefire-plugin/examples/junit.html

On Thu, Jul 19, 2012 at 3:54 PM, Tibor17 <
[email protected]

wrote:

because the
maven-surefire-plugin
does use the same format.

So i wanted to be consistent with maven.

Example of test class and properties in pom.xml:

groupId: maven-surefire-plugin
configuration: test: org/junit/tests/AllTests.java
properties: org.junit.categories.excluded: org/junit/SomeOtherTest.java

But we may change it if necessary.


Reply to this email directly or view it on GitHub:
https://github.com/KentBeck/junit/pull/461#issuecomment-7113457

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jul 20, 2012

In passed, the groups was dedicated only to TestNG. Now also to JUnit.
I will change it to class names.
Do you agree?

@Tibor17
Copy link
Contributor Author

Tibor17 commented Jul 20, 2012

@dsaff
ok, so i modified the implementation, and all four tests passed

* Note that, for now, annotating suites with {@code @Category} has no effect.
* Categories must be annotated on the direct method or class.
*
* <p>
* When filtering every individual test method to run, the child categories are
Copy link
Member

Choose a reason for hiding this comment

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

I'll be honest--I'm keep coming back to review this, and end up balking at this block of javadoc. I think we have to find a clearer way to express what's going on here in order to make it useful to users. Unfortunately, time is pretty tight for me, so it will be a while before I have time to make specific line-by-line suggestions.

I have two thoughts for moving forward:

  • Copy the Javadoc into a Google doc for some collaborative editing as time allows--once we've iterated to a good point, we can copy back here.
  • Ask on the [email protected] list for a collaborator that does have time to give this an edit with a critical eye.

Ideas?

@Tibor17
Copy link
Contributor Author

Tibor17 commented Aug 4, 2012

@dsaff
no problem, i will open new discussions on this, and let you know again after we got coincidence in yahoo and google doc.

@Tibor17 Tibor17 merged commit cac8c34 into junit-team:master Aug 7, 2012
@Tibor17
Copy link
Contributor Author

Tibor17 commented Aug 8, 2012

@dsaff this pull closed by some merge in a mistake :( Can be reopened? The changes are gone , but i have backup, so what should i do? Open a new pull?

@dsaff
Copy link
Member

dsaff commented Aug 9, 2012

I'm sorry, I don't seem to be able to. Can you re-request?

@Tibor17
Copy link
Contributor Author

Tibor17 commented Aug 9, 2012

yes, i will re-request again, and i will let you know what changes i made to simplify javadoc. Your right, the user may really have millions of question in corner behavior -around @ignore and empty tests, etc.
So i will try to write more simple javadoc to read, comparable to other junit javadoc that we have, been more practical.

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.

2 participants