It's high time to introduce some consistency :) #147

bartoszmajsak · 2013-12-03T11:21:50Z

Hi guys,

I've been looking at the tests we have so far and I spotted few things which I believe we should address any time soon to avoid confusion in the future.

We have at least 3 different ways for naming test methods [1] [2] [3]. I personally favor option 3, as test methods tend to be lengthy soReadingThemInTheCamelCaseFormCanBeQuiteTrickyFromTimeToTime. It's also easier to spot them on the stack trace.
As per @aslakknutsen suggestion small README about each and every sample and the goals for tests would be beneficial not only to understand what one (or they...) can find there but also what can be implemented if missing (some hints about missing test cases etc.)
The testing framework: even though JUnit is defacto a standard why couldn't we use Spock? It's not only cleaner, more structured and descriptive way of expressing the tests, but will be also a dog fooding exercise as Arquillian has an extension for that (yeah yeah I do have hidden agenda here). Also when it comes to assertions I would stronly opt for assertj as it's simply way more feature rich and growing very rapidly. On top of that it's easier to write such assertions in the IDE, as it is fluent interface (so we can rely on code completion) rather than onioning in the Hamcrest style.

That said, I would be happy to help in converting/unifying our code base. Let's just agree on sth instead of putting more freestyle code here and there.

aslakknutsen · 2013-12-03T11:27:24Z

@bartoszmajsak Would you be interested in converting one of the existing tests to use Spock so show the difference?

bartoszmajsak · 2013-12-03T11:28:37Z

@aslakknutsen I can contribute new tests written in Spock (eg. JPA stuff). Does it sound good enough?

aslakknutsen · 2013-12-03T11:41:26Z

@bartoszmajsak sure, that'll work too

arun-gupta · 2013-12-03T12:52:56Z

Works for me too. This could be a starting point.

hasalex · 2013-12-03T14:01:47Z

+1 for AssertJ.
Never tried Spock, but would be an opportunity to learn.

kubamarchwicki · 2013-12-03T16:55:52Z

+1 for spock.
Though for people not very familiar with the complete stack (JEE umbrella + concrete spec + Arquillian) there is an entry barrier. That's one of my thoughts after a hack the spec night where I together with a bunch of programmers (gender omitted ;)) tried to add some value with more tests
Even thought people were using parts of the spec on daily basis (servlets, jpa) - getting on track with the project and getting head around Arquillian took us some time. I'd be afraid that adding Spock to this configuration might make cracking the tests even harder.

This is great learning opportunity with Spock but if we are looking for a wider adoption and participation it might get things more messy. Additionally if this is a learning exercise for other I think we should leave this as simple as possible, putting JEE sample + tests as understandable for an average programmer as possible.

bartoszmajsak · 2013-12-03T19:00:24Z

Personally I don't see as a big challenge to grasp Spock. Especially if you know just a bit of unit testing. And at the end of the day the more you learn the better. But I leave it open.

stenaksel · 2013-12-04T06:57:23Z

I really like the "hidden agenda" :-)

radcortez · 2013-12-04T12:32:53Z

Just do a couple of tests using Spock and then people can decide.

Regarding naming conventions for the test methods, option 3 doesn't feel java to me, but if everyone wants to change it, let's go for it.

I do have some additional points about consistency:

Should we start using some common checkstyle rules?
The new files being created don't have the copyright notice (mines included :) ). Should we enforce it?

bartoszmajsak · 2013-12-04T12:38:28Z

Sure, I will throw in some stuff very soon.

With regards to "does not feel java"... I had the same feeling initially, but c'mon, thisIsJustAConventionToImproveReadabilityOfLongAndOverTalkativeMethodNames :) ofCourseIfWeUseSpockThisWontBeAProblemAnymoreBecauseYouWillNameItDifferently. Now that was java ;)

Checkstyle +1
About the copyright - yeah... noticed that as well. I think we can consider to throw it in. Can be automated too.

arun-gupta · 2013-12-04T14:34:10Z

No copyright is needed on new files. The top-level LICENSE file says:

"Except where otherwise indicated, everything in this repository is licensed under the MIT license"

so we are ok.

Checkstyle +1

Do you want to start adding some of these guidelines at http://javaee-samples.github.io/ ?

bartoszmajsak · 2013-12-04T14:38:01Z

Ok, so we might need to review the license headers if they are still relevant in the source files.
I can do some bits of guidelines both on the website and on the README displayed when you land in the repo. Ideally only readme here which can be later on simply imported when generating the website.

arun-gupta · 2013-12-04T14:40:23Z

yep, sounds reasonable. You and Aslak can drive the website content ?

bartoszmajsak · 2013-12-04T14:41:47Z

I can only speak for myself ;) I'm in.

arun-gupta · 2013-12-04T14:42:54Z

Aslak created it, now you can drive it :-)

aslakknutsen · 2013-12-04T14:46:19Z

Working on a extension to extract test results from the Jenkins jobs and categorizing the test cases

aslakknutsen · 2013-12-04T14:46:44Z

@arun-gupta Any chance to get the whole thing relicensed under one license?

arun-gupta · 2013-12-04T15:00:26Z

Would love to, let me find out if that's possible. What would be the benefit ?

aslakknutsen · 2013-12-04T15:11:38Z

@arun-gupta one license to deal with. Not 3.. But beyond just making it simpler, not much.. :)

arun-gupta · 2013-12-04T15:32:42Z

Let me see what can be done now that everything is moved to a separate github organization.

#147 Extended README with some contribution guidelines.

kubamarchwicki · 2013-12-29T17:00:29Z

@bartoszmajsak I withdraw my Spock comment :)
I've fiddled with Spock a bit over past few days and tried adding it to the samples project. While it works fine within IDE, I'm having problems with running it through maven. mvn clean test says there are no tests to run.

And when any tests are picked I got "Multiple TestRunners found, only one allowed. Check your classpath". Any progress since this discussion? Or splitting execution is the only way to go?

I'm referring to these two commits: 517773c and 792c784. Any clue? What am I missing?

bartoszmajsak · 2013-12-29T17:32:20Z

Hey,

By default surefire does not treat *Specification as tests to run. You need
to explicitly set it in the configuration.

HTH
29 gru 2013 18:00 "Kuba Marchwicki" [email protected] napisał(a):

@bartoszmajsak https://github.com/bartoszmajsak I withdraw my Spock
comment :)
I've fiddled with Spock a bit over past few days and tried adding it to
the samples project. While it works fine within IDE, I'm having problems
with running it through maven. mvn clean test says there are no tests to
run.

I'm referring to these two commits: 517773chttps://github.com/javaee-samples/javaee7-samples/commit/517773c3d2c6e8029b0eafbb0c8a2106b86b9840and
792c784792c784.
Any clue? What am I missing?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/147#issuecomment-31320493
.

kubamarchwicki · 2013-12-29T17:37:12Z

Not much :/ Adding Specification to surefire doesn't help - already tried.
Still no test to run. Renaming the class to test - still not picked.
Adding both Test and Specification suffix to inclusions cause the 'two
containers error'

Means I'll have to fiddle a bit with the pom file ad this splitting
execution idea sound to me as a way to go.

On Sunday, December 29, 2013, Bartosz Majsak wrote:

Hey,

By default surefire does not treat *Specification as tests to run. You
need
to explicitly set it in the configuration.

HTH
29 gru 2013 18:00 "Kuba Marchwicki" <[email protected]<javascript:_e({}, 'cvml', '[email protected]');>>
napisał(a):

@bartoszmajsak https://github.com/bartoszmajsak I withdraw my Spock
comment :)
I've fiddled with Spock a bit over past few days and tried adding it to
the samples project. While it works fine within IDE, I'm having problems
with running it through maven. mvn clean test says there are no tests to
run.

I'm referring to these two commits: 517773c<
517773c3d2c6e8029b0eafbb0c8a2106b86b9840>and

792c784<
792c784e7a15378ba401400de3c4fe4a5a5aa7e4>.

Any clue? What am I missing?

—
Reply to this email directly or view it on GitHub<
https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31320493>

.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/147#issuecomment-31321154
.

bartoszmajsak · 2013-12-29T17:39:33Z

Have a look at spock-arquillian module and its examples. Maybe it will shed
some light.

Sorry for being that brief, but replying from mobile is a bit challenging
;-)

Cheers,
Bartosz
29 gru 2013 18:37 "Kuba Marchwicki" [email protected] napisał(a):

Not much :/ Adding Specification to surefire doesn't help - already tried.
Still no test to run. Renaming the class to test - still not picked.
Adding both Test and Specification suffix to inclusions cause the 'two
containers error'

Means I'll have to fiddle a bit with the pom file ad this splitting
execution idea sound to me as a way to go.

On Sunday, December 29, 2013, Bartosz Majsak wrote:

Hey,

By default surefire does not treat *Specification as tests to run. You
need
to explicitly set it in the configuration.

HTH
29 gru 2013 18:00 "Kuba Marchwicki" <[email protected]<javascript:_e({},
'cvml', '[email protected]');>>
napisał(a):

@bartoszmajsak https://github.com/bartoszmajsak I withdraw my Spock
comment :)
I've fiddled with Spock a bit over past few days and tried adding it
to
the samples project. While it works fine within IDE, I'm having
problems
with running it through maven. mvn clean test says there are no tests
to
run.

I'm referring to these two commits: 517773c<

517773c3d2c6e8029b0eafbb0c8a2106b86b9840>and

792c784<

792c784e7a15378ba401400de3c4fe4a5a5aa7e4>.

Any clue? What am I missing?

—
Reply to this email directly or view it on GitHub<

https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31320493>

.

—
Reply to this email directly or view it on GitHub<
https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31321154>

.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/147#issuecomment-31321238
.

kubamarchwicki · 2013-12-29T17:44:25Z

I've started with that one as an example. I suppose mixing junit and spock
containers might be the issue. Will go deeper with this later on.

On Sunday, December 29, 2013, Bartosz Majsak wrote:

Have a look at spock-arquillian module and its examples. Maybe it will
shed
some light.

Sorry for being that brief, but replying from mobile is a bit challenging
;-)

Cheers,
Bartosz
29 gru 2013 18:37 "Kuba Marchwicki" <[email protected]<javascript:_e({}, 'cvml', '[email protected]');>>
napisał(a):

Not much :/ Adding Specification to surefire doesn't help - already
tried.
Still no test to run. Renaming the class to test - still not picked.
Adding both Test and Specification suffix to inclusions cause the 'two
containers error'

Means I'll have to fiddle a bit with the pom file ad this splitting
execution idea sound to me as a way to go.

On Sunday, December 29, 2013, Bartosz Majsak wrote:

Hey,

By default surefire does not treat *Specification as tests to run. You
need
to explicitly set it in the configuration.

HTH
29 gru 2013 18:00 "Kuba Marchwicki" <[email protected]<javascript:_e({}, 'cvml', '[email protected]');><javascript:_e({},

'cvml', '[email protected] <javascript:_e({}, 'cvml',
'[email protected]');>');>>

napisał(a):

@bartoszmajsak https://github.com/bartoszmajsak I withdraw my
Spock
comment :)
I've fiddled with Spock a bit over past few days and tried adding it
to
the samples project. While it works fine within IDE, I'm having
problems
with running it through maven. mvn clean test says there are no
tests
to
run.

I'm referring to these two commits: 517773c<

517773c3d2c6e8029b0eafbb0c8a2106b86b9840>and

792c784<

792c784e7a15378ba401400de3c4fe4a5a5aa7e4>.

Any clue? What am I missing?

—
Reply to this email directly or view it on GitHub<

https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31320493>

.

—
Reply to this email directly or view it on GitHub<

https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31321154>

.

—
Reply to this email directly or view it on GitHub<
https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31321238>

.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/147#issuecomment-31321276
.

bartoszmajsak · 2013-12-29T17:46:15Z

That is probably a problem. Will have a look once close to my laptop :-)
29 gru 2013 18:44 "Kuba Marchwicki" [email protected] napisał(a):

I've started with that one as an example. I suppose mixing junit and spock
containers might be the issue. Will go deeper with this later on.

On Sunday, December 29, 2013, Bartosz Majsak wrote:

Have a look at spock-arquillian module and its examples. Maybe it will
shed
some light.

Sorry for being that brief, but replying from mobile is a bit
challenging
;-)

Cheers,
Bartosz
29 gru 2013 18:37 "Kuba Marchwicki" <[email protected]<javascript:_e({},
'cvml', '[email protected]');>>
napisał(a):

Not much :/ Adding Specification to surefire doesn't help - already
tried.
Still no test to run. Renaming the class to test - still not picked.
Adding both Test and Specification suffix to inclusions cause the 'two
containers error'

Means I'll have to fiddle a bit with the pom file ad this splitting
execution idea sound to me as a way to go.

On Sunday, December 29, 2013, Bartosz Majsak wrote:

Hey,

By default surefire does not treat *Specification as tests to run.
You
need
to explicitly set it in the configuration.

HTH
29 gru 2013 18:00 "Kuba Marchwicki" <[email protected]<javascript:_e({},
'cvml', '[email protected]');><javascript:_e({},

'cvml', '[email protected] <javascript:_e({}, 'cvml',
'[email protected]');>');>>

napisał(a):

@bartoszmajsak https://github.com/bartoszmajsak I withdraw my
Spock
comment :)
I've fiddled with Spock a bit over past few days and tried adding
it
to
the samples project. While it works fine within IDE, I'm having
problems
with running it through maven. mvn clean test says there are no
tests
to
run.

I'm referring to these two commits: 517773c<

517773c3d2c6e8029b0eafbb0c8a2106b86b9840>and

792c784<

792c784e7a15378ba401400de3c4fe4a5a5aa7e4>.

Any clue? What am I missing?

—
Reply to this email directly or view it on GitHub<

https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31320493>

.

—
Reply to this email directly or view it on GitHub<

https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31321154>

.

—
Reply to this email directly or view it on GitHub<

https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31321238>

.

—
Reply to this email directly or view it on GitHub<
https://github.com/javaee-samples/javaee7-samples/issues/147#issuecomment-31321276>

.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/147#issuecomment-31321384
.

kubamarchwicki · 2013-12-29T20:41:18Z

Execution was the problem. Separated executions in same phase but different files and worked (069af04)

arun-gupta added a commit that referenced this issue Dec 4, 2013

Merge pull request #159 from bartoszmajsak/contribution_guide

8b9e0ee

#147 Extended README with some contribution guidelines.

bartoszmajsak added the enhancement label May 4, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

It's high time to introduce some consistency :) #147

It's high time to introduce some consistency :) #147

bartoszmajsak commented Dec 3, 2013

aslakknutsen commented Dec 3, 2013

bartoszmajsak commented Dec 3, 2013

aslakknutsen commented Dec 3, 2013

arun-gupta commented Dec 3, 2013

hasalex commented Dec 3, 2013

kubamarchwicki commented Dec 3, 2013

bartoszmajsak commented Dec 3, 2013

stenaksel commented Dec 4, 2013

radcortez commented Dec 4, 2013

bartoszmajsak commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

bartoszmajsak commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

bartoszmajsak commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

aslakknutsen commented Dec 4, 2013

aslakknutsen commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

aslakknutsen commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

kubamarchwicki commented Dec 29, 2013

bartoszmajsak commented Dec 29, 2013

kubamarchwicki commented Dec 29, 2013

bartoszmajsak commented Dec 29, 2013

kubamarchwicki commented Dec 29, 2013

bartoszmajsak commented Dec 29, 2013

kubamarchwicki commented Dec 29, 2013

It's high time to introduce some consistency :) #147

It's high time to introduce some consistency :) #147

Comments

bartoszmajsak commented Dec 3, 2013

aslakknutsen commented Dec 3, 2013

bartoszmajsak commented Dec 3, 2013

aslakknutsen commented Dec 3, 2013

arun-gupta commented Dec 3, 2013

hasalex commented Dec 3, 2013

kubamarchwicki commented Dec 3, 2013

bartoszmajsak commented Dec 3, 2013

stenaksel commented Dec 4, 2013

radcortez commented Dec 4, 2013

bartoszmajsak commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

bartoszmajsak commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

bartoszmajsak commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

aslakknutsen commented Dec 4, 2013

aslakknutsen commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

aslakknutsen commented Dec 4, 2013

arun-gupta commented Dec 4, 2013

kubamarchwicki commented Dec 29, 2013

bartoszmajsak commented Dec 29, 2013

kubamarchwicki commented Dec 29, 2013

bartoszmajsak commented Dec 29, 2013

kubamarchwicki commented Dec 29, 2013

bartoszmajsak commented Dec 29, 2013

kubamarchwicki commented Dec 29, 2013