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

Add code coverage for integration tests #17

Closed
oskopek opened this issue Dec 26, 2013 · 22 comments
Closed

Add code coverage for integration tests #17

oskopek opened this issue Dec 26, 2013 · 22 comments
Milestone

Comments

@oskopek
Copy link
Owner

oskopek commented Dec 26, 2013

Make cobertura take integration test data too

@sudhirmohanraj
Copy link

can you give a bit more insight into it.

@oskopek
Copy link
Owner Author

oskopek commented Mar 7, 2014

Our tests are being supervised by the cobertura-maven-plugin, then the coveralls-maven-plugin uses it to send the data to Coveralls.io.
The problem is, our integration tests (those running directly in the Java EE container) aren't being supervised by cobertura, therefore not seen in the coverage stats.

The aim is to make cobertura supervise integration tests too.

@sudhirmohanraj
Copy link

gotcha ill look into this.

@oskopek
Copy link
Owner Author

oskopek commented Mar 11, 2014

We should continue all discussions here, so it's in one place. To continue on your line comment:

Yes, cobertura needs to be made aware of integration tests. I don't know how, haven't looked into it - not even sure if it's possible at all.

To give a bit more details, integration tests are mixed in the test directory with unit tests. You can spot an integration test by looking at the class name - it ends with "IT", as opposed to unit tests, which end in "Test". Most of the integration tests are in carcv-webapp, and are run with Arquillian on Wildfly.

As of my recent changes, carcv-core also has integration tests, although not run with Arquillian. Those changes also seem to have cause a problem with Travis I'm not able to fix yet, that's why the tests are "error"-ing. They run fine locally I think.

EDIT: I forgot to mention, integration tests are run through profiles - "it" in carcv-core, and for example "wildfly-it" in carcv-webapp (there are multiple there). If you want to run both, run mvn clean verify -Pit,wildfly-it from the main directory.
They are also run in a completely different part of the maven lifecycle - in phase "integration-test" or "verify".

@oskopek
Copy link
Owner Author

oskopek commented Mar 12, 2014

I've thought about this. Maybe it isn't such a good idea to include integration tests in the code coverage. That should probably just contain unit test coverage, as that's what it was designed to depict.

If we really need the test coverage for IT, we should probably switch to JaCoCo.

@sudhirmohanraj
Copy link

  • That makes sense. I will post any updates on this thread.
  • I looked into it a bit, i saw a few blog posts that said it should be possible, but have not come across a situation where i had to use integration tests to account for code coverage.I will post the helpful links out here on this.
  • Makes sense, I was having a difficult time understanding the structure but now its a little more clearer.
  • I am having a difficult time running the integration tests, have set up wildfly but for some reason it says cannot deploy, will post the stack trace shortly. Trying to understand the IT tests so that i can write a sample scala check code.
  • May be true, when i looked at the link it seems pretty straightforward.
  • Let me see if i can have any luck with cobertura, if i cannot then i will use JaCoCo.

@oskopek
Copy link
Owner Author

oskopek commented Mar 13, 2014

@sudhirmohanraj
Copy link

when i try to run the integration tests with this command; i have set it up pwildfly according to integration tests set up.

screen shot 2014-03-17 at 10 46 19 pm
screen shot 2014-03-17 at 10 41 45 pm

i get the error attached.

The settings are being picked up by the parent project but for some reason the integration tests are failing travis build

working on it.

@oskopek
Copy link
Owner Author

oskopek commented Mar 18, 2014

I've recently done some work on integration tests. You should pull the newest changes from develop as soon as you can.
If you're trying to exchange cobertura for JaCoCo, I'm not sure we need to. As I said above:

  1. I'm not sure about integration tests being counted into the coverage percentage
  2. JaCoCo can't handle branching coverage and line coverage counting AFAIK

Nevertheless, you should be able to run integration tests from the newest commit onward. If you get any errors after pulling, paste the whole log somewhere and we can work from there.

EDIT: I reran the build, everything seems to be fine. Passes locally too. If it doesn't work, tell me how you set up your environment and what OS / Java version you use.

@sudhirmohanraj
Copy link

JaCoCo seems to be able do it. Since we already have cobertura picking up the unit tests. I will try to see if it can pick up.

  • You were right now the remote build is passing after rebase.
  • But my local build still seems to fail. instructions i followed this to set up.
    1. but it again seems to give me the same problem as pasted above. I must be missing something very basic.
    2. i have java 7 and mavericks os on mac.

@oskopek
Copy link
Owner Author

oskopek commented Mar 19, 2014

Ok. Can you paste the whole log somewhere? (From Maven)

@sudhirmohanraj
Copy link

log this is the log message.

  • I just had a general question, how accurate is coveralls, because when i give the same commit from develop without any changes it still shows a coverage less than develop branch.
  • Is there a time latency before coverage information can be trusted?

@oskopek oskopek removed the easy label Mar 20, 2014
@oskopek
Copy link
Owner Author

oskopek commented Mar 20, 2014

  1. The Coveralls question: I have no idea how they update the banner, but it seems to be pretty unreliable and not up to date. The latest coverage seems to be at around 32%. No idea how often and how they update.
  2. Regarding the log: I realized the docs were really misleading and to prevent future miss-directions like this, refactored and rewrote them today. The thing you are missing is a security-domain in wildfly's standalone.xml. Also, check the updated document on setting up the build environment, if you are missing something.
    The hint in the error log was:
Caused by: java.lang.Exception: {"JBAS014771: Services with missing/unavailable dependencies" => ["jboss.undertow.deployment.default-server.default-host./carcv-webapp.UndertowDeploymentInfoService is missing [jboss.security.security-domain.carcv]"]}

Also, the cobertura checks fail. If you enabled them, it's quite expected -- they are set too strictly for our coverage right now.

@sudhirmohanraj
Copy link

standalone.xml; fixed the problem.

@sudhirmohanraj
Copy link

commit this should fix it. I am not sure why it fails when we have this phase enabled. At this time i am running out of ideas.

@oskopek
Copy link
Owner Author

oskopek commented Mar 28, 2014

Are you sure we want to instrument in the process-classes phase? I changed it to f.e. package and the tests passed. I'm not sure I "fixed" it correctly - think you had a motivation for the process-classes phase.

@sudhirmohanraj
Copy link

link this pom file has the same use case.
gist of the above linkusage.

  • First we need to instrument the classes.
  • Second we need to jar up the instrumented classes and have them used by the build later.
  • Need to tell the Integration Tests to use the instrumented classes for it’s dependencies.

@oskopek
Copy link
Owner Author

oskopek commented Mar 28, 2014

Sounds right. Last time I checked, you were just instrumenting them, right? Wouldn't that cause the tests to fail, because you aren't packaging them into the war file?

EDIT: Also, I'm not really comfortable with the thought of testing only the instrumented classes. We should test "normal" production code too.

@sudhirmohanraj
Copy link

ok. i wont follow that approach. I will keep this on hold and start with another feature; come back to this at a later point.

@oskopek
Copy link
Owner Author

oskopek commented Mar 28, 2014

I agree. Keep the code & ideas, we might need that later on.
One solution to running instrumented and not instrumented classes in tests on their own would be to create separate profiles, but lets leave it as is for now.

@oskopek
Copy link
Owner Author

oskopek commented Mar 28, 2014

I'm closing the issue as wontfix for now in order to get back to it eventually.
Thank you for all the work and time you've put into this!

@oskopek oskopek closed this as completed Mar 28, 2014
@sudhirmohanraj
Copy link

ok that sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants