Configurable Categories

[Tibor17]

Categories in suite should be multiple in annotations Categories.IncludeCategory and Categories.ExcludeCategory, and configurable by system properties as well.
This pull request referst to discussions in #142 and my old pull request #354 closed by my mistake #461.

[Tibor17]

@dsaff can you please check the javadoc again. Now it should be ok.

[dsaff]

Please don't change the import order.

[Tibor17]

I will fix it.

[Tibor17]

@aslakknutsen
@matheeeny
@stefanbirkner
@langles
You participated in our feature 'support for multiple Include/Exclude categories'.
I forked that feature to #503.
David @dsaff thinks that the javadoc is incomplete.
It's our common feature, thus can you please read this javadoc in Categories class, and briefly write in a comment the javadoc that you want.

[Tibor17]

i have got a response from junit users, so i will update the javadoc accordingly

[dsaff]

Tibor,

Do you think there's still momentum around this?

[Tibor17]

@dsaff
yes, i am only struggling with git merge in other pulls.
I want to update the javadoc, definitelly will do it next week.
Since you want to release 4.11 soon, we do not have time to introduce this in 4.11, but having it in 4.12 is realistic from my point of view because i plant to allocate my time to this during the weekend and next week.

[Tibor17]

I will push new javadoc after Friday.

[Tibor17]

@krosenvold

This pull is also intended for surefire. The point is to specify more than one category.
Using this feature, both TestNG and JUnit would provide multiple categories.

I would simplify the Javadoc and update JUnit wiki with more details.
Are you interested in this feature as well?
Currently the CategoryFilter is used in JUnit48Provider. Maybe a new provider without surefire's CombinedCategoryFilter would be needed in my guess.

In this feature, we can include only tests annotated by SlowTests AND FastTests.
We can also exclude tests annotated by SlowTests AND FastTests.
The OR alternative exists as well.

Example:

CategoryFilter slowOrFast = CategoryFilter.include(Selection.ANY/means OR/, SlowTests.class, FastTests.class);
Request baseRequest= Request.aClass(OneOfEach.class);
Result result= new JUnitCore().run(baseRequest.filterWith(slowOrFast));

Similar with #exclude().

(without using the object Request, we can do the same using system properties)

An example with a combined filter :
CategoryFilter#categoryFilter(Set<Class> inclusions, Selection inclusionsSelect, Set> exclusions, Selection exclusionsSelect)

Unless the Selection is specified, a default value is taken ALL for included and ANY for excluded.
I think extra two params in surefire plugin would be nice to have for this reason.

For more info regarding factory methods in class CategoryFilter:
https://github.com/Tibor17/junit/blob/3135e346251ac83b633bca737f72e03313cbdbbf/src/main/java/org/junit/experimental/categories/Categories.java

Thx, Tibor

[gaffa]

I have realized this is about more than what was asked two years ago.
Anyway. I still need @IncludeCategory and @ExcludeCategory to support Class<?> []
Shall I open yet another pull request for this feature (i think there were already like a dozen)?

[Tibor17]

@gaffa
It does not make sense to open a new pull request because we have Class<?> [] in @IncludeCategory and @ExcludeCategory.
The best power is to join together rather than to split our individual activities.

We should now refactor the Javadoc and find out comprehensive text and tests covering the basic principle.
The tests are available. The only thing is to provide a clear and simple explanation -why do i need this runner and how to use it.
The detailed description would go to our wiki page.

I will concentrate on tomorrow.

[gaffa]

I can see your point. But joining Tasks may be the reason why the feature isnt yet included eventhough this was requested two years ago.
Also It does not feel right trying to understand all your changes and write Javadoc for it. By the way... there is nearly no javadoc in Categories-Code at all. Why being so harsh on your changes?
Also you have to understand my situation. I am a full-time-developer with like a hundred projects already waiting for me in my spare time. I am currently managing test-setup on a project with around 10k+ tests and need to find solutions. They even might concider switching to testng if I dont come up with those. As you can see for me the topic is not academic but strictly harming my productivity. Waiting two years more is not an option ;)
Anyway: thank you for looking into the topic. If theres anything precise I can do I will be happy to contribute. Maybe open a few Issues in the tracker?

[Tibor17]

No, it's not that reason.
The only reason is that we have many pull requests open (parallel computer, mavenize, osgi, etc.).

[dsaff]

@gaffa,

Here's one way you can help: imagine the implementation that you would want. Write the javadoc for that imaginary implementation. Think about every combination of possibilities.

If, by luck, your desired implementation matches what @Tibor17 has built, then we can use your docs and go for it. If not, it may provide an alternate starting point for edits.

Do you have time to help in that way?

[gaffa]

Javadoc for what, btw? The change that I am keen on is only in ExcludeCategory and IncludeCategory and changes the description from Class to Class[]. Are you talking about the whole categories-Package?

[Tibor17]

@gaffa
If you really want to help us, then better to start by launching the tests we added. We basically populate here tests given by the contributors.
Also the reasons why and the way how we extended the annotations IncludeCategory and ExcludeCategory can be found in the issue #142.

[gaffa]

I have studied that thread and all that I can find is that other related stuff got stuffed into the topic.
Ill fork your repo soon and have a look at it.

"the tests" and "javadoc" still is damn inprecise ;)

[Tibor17]

@gaffa
That's the reason why we need the Javadoc updated.
The tests should fine!

[gaffa]

I had a look into it just now. You will not like what I have to say. First of all the ALL/ANY concept is exactly what people would expect. I do not see a big benefit in making that optional (as its not a good idea to build such a complex test setup anyway) but you add a lot of complexity by doing so. Also ALL and ANY are not intuitive. In my opionion the possibility of building such a test setup is an over-engineered approach.

@category({IT.class, Slow.class}
class A{}

@category(IT.class)
class B{}

@category(Slow.class)
class C{}

Whats the use in just running A? If I want to run classes that contain Integration-Tests and Slow-Tests why should I want to not run B or C? Makes no sense to me. You should find a use-case and illustrate it before adding such a feature.

So much for Selection. Removing it would reduce the complexity a lot (with capital letters). As for the rest of the changes I am having another problem. Letting aside your changes I already think that Categories.java is coded in a way that is not easy to adopt. Easy to read-code, easy to adopt-design and common patterns are preferable to enable people to understand whats going on. Jumping into Categories does not give a good experience to the user at all. Your additions did not make that better. Its pretty clear you are a good coder but some patterns should be avoided when developing Java-code. Good code today is easy to understand at first sight and not reduced to the minimum using constructs only 10% of the people understand, using short field-names, few comments, ...
By the way: your tests pretty much illustrate what I am talking about. Tests should illustrate the usage of the api pretty easily but even they are hard to get.

My conclusion is that what happened here is exactly what i assumed before seeing the code. Too much got mixed in it and that resulted in a complex and hard to include result. For me that means that I think I will provide another pull request that introduces the concept of class-Arrays without the complexity of Selection. Really I am sorry to tell you but as far as I am concerned your solution is way to complex. If you really come up with a use-case for Selection you urgently need to work on the design to make it less complex. The concept is mixed in the code everywhere. You will need to encapsulate it somewhere else. Eg a service. The logic mixed in Categories needs to be stuffed in "generic" methods with documentation and names that are called from Categories to make clear whats going on.

Do not be f* off by my comments and the fact that I am not aboard. I mean well. Maybe someone from the outside looking into it and giving honest and detailled feedback also helps.

[Tibor17]

As in #142, this is what we agreed we want.
Before we had array-based implementation which I could not accept because it was a bad approach because of the Class duplicates.
I think the implementation is readable and can be used in maven-surefire project.

Again the most important thing is now the Javadoc to concentrate on.

We would need the ANY/ALL to satisfy two parties of users. This is what we understood in #142. There we were exactly two groups.

I have also new tests which improve the Javadoc, but no time to finish it.
The complete description of Selection use would need categories inheritance.
I would like to write Javadoc with a simple use case, and usage page in JUnit wiki with more details. So that the Javadoc would link to our wiki page. This is the approach we agreed in othe pull with ParallComputer.

If I have to submit a change, then it has to satisfy more than your needs. We are not developing for one user, right?

[gaffa]

Yeah, right. I did not see that there was a pull request with the simple approach already that was discussed until it died. But in the end JUnit is for the developers using it, not those developing it. And the behaviour that all Classes specified in the Annotation is what is expected. Really. :)
I would suggest accepting the simple solution as this means a fast benefit for everyone. In the end Selection is a compatible addition and not a different approach.

[Tibor17]

@gaffa
I do not think this is normal approach of collaboration. Better the community knows what feature they request.
Again we do not develop for an individual person. Now it would only satisfy only your needs and not the others who started this activity.
I hope you are aware of.

[gaffa]

I have to disagree. This is an effective compromise which is 100% compatible to your approach. Its just a way of getting a feature in without waiting for you sorting things out. It has already been two years. And you really cannot expect me to fix your documentation or to finish your feature (which again: is a compatible addition). I have spent time looking in your stuff and given you feedback. Afterwards I decided that this feels far from finished and suggested a compromise that allows you to work on it when you find the time. I think that is a perfectly fair solution. Talking about the communty: do you really think that they would have not liked to have the feature without Selection 23 months ago rather than waiting until ...? Also: the feature is 100% backwards compatible and 100% compatible to your feature. I dont get the problem.
Its not about satisfying my needs. Its about adding a feature that is REALLY (i did it, capital letters, shoot me) needed by the community. Like really. Not like nice to have.

Also: the inital request was exactly what I implemented. You rocket-sciencing it does not mean that request is not as valid as it was 2 years ago ;)

[gaffa]

To gain peace I have decided to implement your feature in a way I think it makes the most sense (I dont think anyone will use it but anyway. I dont want to be blamed for only representing my interests again). Its not finished. I just spent <1 hour at work. But maybe it gives you a hint at how to simplify your feature.

https://github.com/gaffa/junit/commit/99fcd8ee32de4b8c7e1bafdc18946ac56db5c6a5

Still there are three methods that I would extract to a Utility-Class as they pretty much do generic operations (the Class<?> - stuff)

[caiwenhua1]

的很贵

[Tibor17]

@dsaff
I have simplified the things:

• removed system properties and a lot of related code -I can live without that;
• removed most of the javadoc and really made it tiny and clear;
• ready for the most usual scenarios.

I still prefer using Set-s instead of arrays or List-s because class duplicates are not welcome.
We are not doing this: passing arrays by references like in #340 and #565 becaue it is not safe. We make copy here.

Failsafe against null in annovation values.

Ready for use in other open source projet maven-surefire because of CategoryFilter factories.

[gaffa]

Cool. Would love to see it merged in soon.

[Tibor17]

Now the indentation is fixed.
Previously the code was a mixture of tabs and empty chars.
Also Javadoc had one char indentation.
Compared old and new file copy, both match. Tests passed.

[gaffa]

Offtopic: Is there a complete styleguide for junit anywhere (indentation, brackets, imports, ...)? Maybe I will find time to configure intellij and export the settings. Someone could do the same for netbeans and eclipse. Would make things a lot easier for new people. Default IntelliJ-settings are completly different than junit style and thats why I cant use autoformat at all.

[Tibor17]

Fulfilled all previous requests from @dsaff and going to merge with master.

[Tibor17]

@dsaff

This branch has wrong parent, so cannot be merged.

I merged these files in fresh branch in pull #568.

Sorry for the inconvenience.

[Tibor17]

@gaffa
Yes there is a styleguide file CODING_STYLE in the root.

[gaffa]

Thanks!

[dsaff]

@Tibor17, what do you mean "This branch has wrong parent, so cannot be merged"? There's so much history on this pull that I'd love to get the original merged, unless its' just become prohibitive. What happens when you try to merge?

[Tibor17]

I updated my project. I found out that the changes come from branch gh-pages, without latest changes in 4.12.
If you can update that branch, then it would be possible for me to update my branch to the latest changes.
I do not know how it could happen, maybe when i tried the git commands three months ago.

[Tibor17]

@dsaff
@gaffa
Is there a way to configure other remote master branch?

[Tibor17]

@dsaff
I found the way. I will fetch the chages directly from KentBeck, reset the head to the last commit, and then merge.
I think this will be fine!

[Tibor17]

@dsaff

Done

[Tibor17]

@dsaff

all tests passed

[dsaff]

Very glad to see this one reach the finish line! Thanks!

[dsaff]

Can you take a stab at the release notes, as well?

[gaffa]

Yay! (and good night everyone ;))

[Tibor17]

@dsaff
@gaffa

:)))))

Thanks, thanks, thanks.

Sure, i'll go for it!

[Tibor17]

@aslakknutsen
@matheeeny
@stefanbirkner
@langles

To all contributors in #142 regarding multiple categories.

[ajmath]

Congrats on pushing this through guys. We'll be glad to be back on the
upstream JUnit.

Big props to @gaffa for bringing this back to life and @tbor17 for
contributing the bulk of the work throughout the life of this issue

On Monday, December 10, 2012, Tibor17 wrote:

@aslakknutsen https://github.com/aslakknutsen
@matheeeny https://github.com/matheeeny
@stefanbirkner https://github.com/stefanbirkner
@langles https://github.com/langles

To all contributors in #142 https://github.com/KentBeck/junit/issues/142regarding multiple categories.

—
Reply to this email directly or view it on GitHubhttps://github.com/KentBeck/junit/pull/503#issuecomment-11224682.

Andrew Matheny

[kcooney]

BTW: this change removed a public constructor. I'm putting together a pull to add it back.