Pull Requests/Merge to master

[bartoszmajsak]

Hi guys,

I understand we gave up on rebase approach and we do merge from GitHub. Fine with me.

I'm just wondering why do we merge PR which leads to failing build? Such approach has a risk of introducing lots of issues to the whole project because we are missing an opportunity to validate it.

I assume a PR is at least executed on local env of the approver before hitting a magic button, isn't it?

[arun-gupta]

@aheusingfeld any idea why Travis build is failing?

[arjantijms]

I'm just wondering why do we merge PR which leads to failing build?

@bartoszmajsak

I think the reason for failing the build is not correct.

The build is now failed when any test fails. But the tests point out incompatibilities/bugs in various servers, so almost by definition not all tests can pass.

For instance, no matter the commit, the build is now ALWAYS failed because some early test in Batch fails. But if JBoss (which is used to run the tests against I think) itself has a bug here, then the test is still valid, and JBoss should fix it. Meanwhile, the build should not fail here.

The build should for a PR probably only fail if it doesn't compile.

I assume a PR is at least executed on local env of the approver before hitting a magic button, isn't it?

I personally test every PR indeed, but what do you want to see as a result? That's my question. A failing test does not mean the PR is not valid.

[arjantijms]

@arun-gupta

any idea why Travis build is failing?

It seems it actually runs the tests and aborts on the first failure, then marks the entire build as failed. See my comments here as well: #303

[bartoszmajsak]

For instance, no matter the commit, the build is now ALWAYS failed because some early test in Batch fails. But if JBoss (which is used to run the tests against I think) itself has a bug here, then the test is still valid, and JBoss should fix it.

Which essentially means that we (and people potentially interested in Java EE goodies) have no idea if this code is valid or not unless they somehow manage to run relevant parts.

I think the problem lies in the way how the repository is structured, but also how we have builds defined. I believe we can do way better.

At this point all of the infrastructure around serves no purpose in my humble opinion.

[arjantijms]

Which essentially means that we (and people potentially interested in Java EE goodies) have no idea if this code is valid or not unless they somehow manage to run relevant parts.

No, the test is valid. Their server is not.

It points out to people they can't use that feature on their specific server. To run all tests and don't have the run abort on the very first failure --fail-at-end can be used.

That said, we should check external and each other's commits to really make sure the test is indeed valid. This is not always easy, I mean who knows enough about JASPIC here to double check that my JASPIC tests are valid? Again, a failing test does not indicate an invalid test.

I personally go to great lengths to check my tests, and validate with the spec lead of JASPIC in case of doubts and run them on multiple servers. In some cases a seemingly simple commit has taken weeks of testing the test itself.

I would like an easier way to run just the JPA tests or just the JASPIC tests though. Now the easiest way seems to be to just comment out unneeded modules in the main pom.xml file.

[aheusingfeld]

any idea why Travis build is failing?

It seems it actually runs the tests and aborts on the first failure, then marks the entire build as failed. See my comments here as well: #303

That's not totally correct. It seems that Travis builds cannot have more than 10000 lines of output. If you take a look at all the builds, they are all stuck at this log line no matter what it is. I'll ask the travis guys about it.

ah, and by the way

I think the problem lies in the way how the repository is structured, but also how we have builds defined. I believe we can do way better.

I'm 100% with Bartosz on that one. I think it'd be much better if we create separate builds for the sub modules and the builds would also be way faster.

[arjantijms]

It seems that Travis builds cannot have more than 10000 lines of output. If you take a look at all the builds, they are all stuck at this log line no matter what it is.

Okay, that's interesting. It looked like the abort was on the first failure, but could be a coincidence then.

[aheusingfeld]

intermediate result: downloading dependencies alone takes ~3 minutes! https://api.travis-ci.org/jobs/64117841/log.txt?deansi=true

�[31;1mTimeout (20 minutes) reached. Terminating "mvn -q --fail-at-end verify -Ptomee-embedded-arquillian"�[0m

travis_time:end:092faa74:start=1432656942261690260,finish=1432658142724578223,duration=1200462887963
�[0K
�[31;1mThe command "travis_wait mvn -q --fail-at-end verify -Ptomee-embedded-arquillian" exited with 1.�[0m

Our build has been killed after 20 minutes, probably because of the 'quiet' flag. I will remove the flag again so travis shows the real failure and I'll also add the --fail-at-end option so that all tests are run. Nevertheless we should think about splitting the build to at least get faster results.

[aheusingfeld]

There are really a lot of broken tests in the Travis build. Maybe one of you already wants to take a closer look into them while I'm trying to get the build working? https://api.travis-ci.org/jobs/64335298/log.txt?deansi=true

[arjantijms]

One additional type of "failure" that I see in the log is that some tests are run against TomEE, but TomEE is a web profile+ implementation and doesn't support all technologies that are tested.

For instance, JASPIC fails on TomEE, but TomEE doesn't support JASPIC.

[aheusingfeld]

FYI I split the travis build by subfolder so they could theoretically run in parallel. I think the output of this very clearly highlights our current problem: it's just too huge to be one piece! If the build of a single subproject takes more than 6 minutes, how are we still able to speak of "fast feedback"?

@bartoszmajsak I guess splitting by subproject is a reasonable first step. We now need to see whether we can limit builds only to the subprojects where changes have been made. The obvious way would be to really split the github project but I don't like this too much as it has (organizational) side effects.

If none of you minds, I'd like to focus on splitting only the build and try to filter the subprojects via Travis' explicit include feature. If this doesn't work, we can still think about plan B. Does that make sense?

[kubamarchwicki]

Split by subprojects makes a perfect sense.

Though I have a different question, we are testing against wildfly-embedded-arquillian (8.2.0.Final). Not that I have anything against Wildfly, but why aren't the tests run against Glassfish (afterall, the reference implementation)?

[aheusingfeld]

@kubamarchwicki forget what I said in the comment above, I tried different things locally and I think that splitting into multiple subprojects, i.e. javaee7-jpa-samples, is the only right way to go.

Not that I have anything against Wildfly, but why aren't the tests run against Glassfish

I have no feelings about this. I doubt though that switching the server will have an impact on the OutOfMemoryError we currently have for the JPA build :-o