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

Some unit tests are failing in certain machine environments (OS, Maven and Java versions) #764

Closed
renatorroliveira opened this issue Aug 28, 2014 · 39 comments
Assignees
Labels

Comments

@renatorroliveira
Copy link
Collaborator

In some cases, some unit tests are failing.
This failure occurs even on some Travis CI builds.
The result output for the error is:

Results :

Tests in error:

shouldNotRunVRaptorStackIfVRaptorRequestStartedEventNotFired(br.com.caelum.vraptor.ioc.RequestStartedFactoryTest): WELD-001303: No active contexts for scope type javax.enterprise.context.RequestScoped
  shouldRunVRaptorStackIfVRaptorRequestStartedEventIsFired(br.com.caelum.vraptor.ioc.RequestStartedFactoryTest): WELD-001303: No active contexts for scope type javax.enterprise.context.RequestScoped
  shoudRegisterResourcesInRouter(br.com.caelum.vraptor.ioc.cdi.CDIBasedContainerTest): You shouldn't add more interceptors after ordering. Please notify vraptor developers.
  shouldReturnAllDefaultConverters(br.com.caelum.vraptor.ioc.cdi.CDIBasedContainerTest): You shouldn't add more interceptors after ordering. Please notify vraptor developers.
  shouldReturnAllDefaultDeserializers(br.com.caelum.vraptor.ioc.cdi.CDIBasedContainerTest): You shouldn't add more interceptors after ordering. Please notify vraptor developers.
  shoudRegisterConvertersInConverters(br.com.caelum.vraptor.ioc.cdi.CDIBasedContainerTest): You shouldn't add more interceptors after ordering. Please notify vraptor developers.
  shoudRegisterInterceptorsInInterceptorRegistry(br.com.caelum.vraptor.ioc.cdi.CDIBasedContainerTest): You shouldn't add more interceptors after ordering. Please notify vraptor developers.

Some "mvn -V" outputs where the errors occurs:

Apache Maven 3.2.2 (45f7c06d68e745d05611f7fd14efb6594181933e; 2014-06-17T10:51:42-03:00)
Maven home: /opt/maven
Java version: 1.7.0_65, vendor: Oracle Corporation
Java home: /usr/java/jdk1.7.0_65/jre
Default locale: pt_BR, platform encoding: UTF-8
OS name: "linux", version: "3.11.10-21-desktop", arch: "amd64", family: "unix"
Apache Maven 3.2.1 (ea8b2b07643dbb1b84b6d16e1f08391b666bc1e9; 2014-02-14T14:37:52-03:00)
Maven home: /opt/maven
Java version: 1.7.0_60, vendor: Oracle Corporation
Java home: /opt/jdk1.7.0_60/jre
Default locale: pt_BR, platform encoding: UTF-8
OS name: "linux", version: "3.13.0-34-generic", arch: "amd64", family: "unix"

The "mvn -V" output where the error don't occurs:

Apache Maven 3.2.2 (45f7c06d68e745d05611f7fd14efb6594181933e; 2014-06-17T10:51:42-03:00)
Maven home: /opt/apache-maven-3.2.2
Java version: 1.8.0, vendor: Oracle Corporation
Java home: /opt/jdk1.8.0/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "3.14.8-200.fc20.x86_64", arch: "amd64", family: "unix"
Apache Maven 3.1.0 (893ca28a1da9d5f51ac03827af98bb730128f9f2; 2013-06-27 23:15:32-0300)
Maven home: A:\Programming\apache-maven-3.1.0
Java version: 1.7.0_11, vendor: Oracle Corporation
Java home: C:\Program Files\Java\jdk1.7.0_11\jre
Default locale: pt_BR, platform encoding: Cp1252
OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
@Turini Turini added the bug label Aug 29, 2014
@renanigt
Copy link
Contributor

I don't know if here is the correct local to report this, but Travis Ci Build of VRaptor project is failed.

@renanigt
Copy link
Contributor

Sorry, now is passing. 😃

@renanigt
Copy link
Contributor

I think we having some problems with VRaptor tests or Travis Ci, since sometimes the travis ci build is failed also on github VRaptor4 project, not only in some environments.

And always the the test brokes on CDIBasedContainerTest.

What do you think ? @Turini @lucascs @garcia-jj

@Turini
Copy link
Member

Turini commented Aug 30, 2014

yes, this is the reason we open the issue :)

@lucascs
Copy link
Member

lucascs commented Aug 31, 2014

So we probably have an implicit order dependency that makes the test fail.

"You shouldn't add more interceptors after ordering. Please notify vraptor developers."

It seems like a VRaptor 3 inheritance. Which class is throwing this?

@renanigt
Copy link
Contributor

@lucascs, the problem occurs on CDIBasedContainerTest class.

@lucascs
Copy link
Member

lucascs commented Sep 1, 2014

I mean the class that is throwing "You shouldn't add more interceptors after ordering. Please notify vraptor developers."

@renanigt
Copy link
Contributor

renanigt commented Sep 1, 2014

True, sorry @lucascs.

The class is Graph.

@renanigt
Copy link
Contributor

renanigt commented Sep 1, 2014

Testing using eclipse, If we run CDIBasedContainerTest class alone, the error doesn't occur, but running all tests together, the error occur.

@lucascs
Copy link
Member

lucascs commented Sep 1, 2014

@renanigt
Copy link
Contributor

renanigt commented Sep 1, 2014

@lucascs we need call registermethod ? Sorry if I didn't understand you.

@lucascs
Copy link
Member

lucascs commented Sep 1, 2014

No, reinitialize it on CDI.

It seems that that class is being shared between test cases...

put a println or breakpoint on TopologicalSortedInterceptorRegistry's constructor and see how many times it is called during a test run.

@renanigt
Copy link
Contributor

renanigt commented Sep 1, 2014

@lucascs 14 times.

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

@lucascs how can I reinitialize it ?

@lucascs
Copy link
Member

lucascs commented Sep 3, 2014

this 14 times is only running the CDIxxxTest? or the whole src/test/java folder?

https://github.com/caelum/vraptor4/blob/master/vraptor-core/src/test/java/br/com/caelum/vraptor/WeldJunitRunner.java

try to start WeldContainer per test (into createTest)

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

14 times in the whole src/test/java folder.

I'll try it.

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

@lucascs If I start WeldContainer into createTestas you said, the CDIBasedContainerTest pass with success, but break the test shouldUnproxifyCDIProxies on CDIProxiesTest class.

@Turini
Copy link
Member

Turini commented Sep 3, 2014

we should to start the container one single time for all tests

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

I really don't know why this is happening. Even why CDIBasedContainerTest whe is run alone the test pass, but running all tests together, the tests break.

Thanks @Turini and @lucascs ! 😃

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

Just a note: In older versions I didn't have these errors.

IMHO this issue is too important, since new committers may got these errors and may discourage them to commit. Also sometimes the Travis Ci Build on VRaptor github page is failing, and doesn't sounds good, especially for new users.

@garcia-jj
Copy link
Member

To avoid flooding, you can add all text you want in the same comment. So we
receive one email per each comment.

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

Sorry. I did that beause I just remembered after I wrote the other comment.

@lucascs
Copy link
Member

lucascs commented Sep 3, 2014

@Turini why instantiating only one container?

what about applicationScoped components?

@renanigt
Copy link
Contributor

renanigt commented Sep 3, 2014

Since TopologicalSortedInterceptorRegistry is ApplicationScoped, when CDIBasedContainerTest is run, the orderedList from Graph isn't null.

So, here is thrown the message:
checkState(orderedList == null, "You shouldn't add more interceptors after ordering. Please notify vraptor developers.")

Or am I wrong ?

@garcia-jj
Copy link
Member

@lucascs and @Turini All comments about this issue makes sense. But I'm curious why this not happen in some machines (mine as example), and why only some days ago Travis is getting errors (not always). Too strange.

@renatorroliveira
Copy link
Collaborator Author

This bug is related to the order of execution of the tests.
In some orderings of the executions the bug occurs and in some others it doesn't.
I don't know exactly how the surefire orders the tests, but if there is a way to specify in which order the tests should be executed, this can be used to solve this problem.
It seems that in the same machine/environment the order don't change from an execution to another, that could be the reason why it never occurs in your machine and always occurs in other ones.

@csokol
Copy link
Contributor

csokol commented Sep 4, 2014

That's strange because we are using junit 4.11 and the test execution order
should be the same for every environment:
https://github.com/junit-team/junit/wiki/Test-execution-order

Maybe the order of the classes are being mixed up by surefire (since we are
not fixing the surefire maven plugin version in our pom.xml, the actual
plugin might be different in our environments).

I think that if Renato send us the full output of mvn test we can discover
if it is running in a different order from our environments.

Anyway, depending on the test execution order is always bad, I think the
suggestion of @lucascs makes total sense. @Turini, why we shouldn't
instantiate one container for each test class?

Chico Sokol

On Wed, Sep 3, 2014 at 9:58 PM, Renato R. R. de Oliveira <
notifications@github.com> wrote:

This bug is related to the order of execution of the tests.
In some orderings of the executions the bug occurs and in some others it
doesn't.
I don't know exactly how the surefire orders the tests, but if there is a
way to specify in which order the tests should be executed, this can be
used to solve this problem.
It seems that in the same machine/environment the order don't change from
an execution to another, that could be the reason why it never occurs in
your machine and always occurs in other ones.


Reply to this email directly or view it on GitHub
#764 (comment).

@Turini
Copy link
Member

Turini commented Sep 4, 2014

sorry my delay guys

@Turini why instantiating only one container?

the idea is to start the container each test? why we should do that?
it takes more than 1 min to start and shutdown Weld, imagine doing
it each test... i don't think it would be nice :(

what about applicationScoped components?

sorry, I didn't get the point... starting and shutting down the weld
container will not start and stop the scope context... we control it
using the Contexts class... so, if we want to test app scoped beans
we can start, stop, restart, etc the scope on the test...

@Turini
Copy link
Member

Turini commented Sep 4, 2014

Anyway, depending on the test execution order is always bad

this is true... if our tests are broken because of order, we should fix it

@luiz
Copy link
Contributor

luiz commented Sep 4, 2014

I see that we're using a custom runner to start Weld and inject dependencies. I think it's usually better to use a library where possible, instead of reimplementing it. In this case, I think we're reimplementing CDI-Unit (http://jglue.org/cdi-unit/). What do you think of using it instead of maintaining our own JUnit runner? Maybe this will solve the problem.

@Turini
Copy link
Member

Turini commented Sep 5, 2014

Sounds like good to me. I'll try to push a branch today using CDI-unit to test it (and the instabilities problems). Thks @luiz

@Turini Turini self-assigned this Sep 11, 2014
@garcia-jj
Copy link
Member

Travis are bothering us. What you think to restart CDI container on each test (as @renanigt suggested) until we can fix this case? So Travis will like a charm, slow, but all tests will pass.

@Turini
Copy link
Member

Turini commented Sep 17, 2014

I'll try to fix this issue again (I think now I understand whats happening here).
If it does not work (tomorrow) we can restart the container waiting for a better fix.

@garcia-jj
Copy link
Member

Only a note: I see the last change works, but from now tests are too slow. This is not a big problem.

@Turini
Copy link
Member

Turini commented Oct 8, 2014

@renanigt it still failing on your machine? anyone else?

@renanigt
Copy link
Contributor

renanigt commented Oct 8, 2014

Yes @Turini. =/

@Turini
Copy link
Member

Turini commented Oct 28, 2014

I think we can close here, at least until someone else report the same
problem (we can't reproduce). What do you think? It's ok for u Renan?

@renanigt
Copy link
Contributor

Since it's too specific, I agree with you @Turini. Today I can take a look in this problem and try to fix it.

@Turini
Copy link
Member

Turini commented Oct 28, 2014

Thank you @renanigt.
We can reopen this issue any time, if its causing troubles :)

@Turini Turini closed this as completed Oct 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants