CCI-MOC / hil

Hardware Isolation Layer, formerly Hardware as a Service
Apache License 2.0
24 stars 54 forks source link

Add code coverage with coveralls.io #957

Closed xuhang57 closed 6 years ago

xuhang57 commented 6 years ago

Relates to: https://github.com/CCI-MOC/hil/issues/535 https://github.com/CCI-MOC/hil/issues/516

naved001 commented 6 years ago

our coverage is 10.028 %? that seems inaccurate.

xuhang57 commented 6 years ago

I believe a reason why is that it is testing all branches. Still trying to figure out the root cause.

xuhang57 commented 6 years ago

So, I don't know why we had disabled the code coverage tests before.

But it turns out, If I enable it, in Travis (not through the setup.cfg), lots of tests are failing.

Now, it would be great if someone could help me with:

  1. why enable the coverage test in setup.cfg?
  2. why these tests are failing, after enabling the coverage tests?

Thanks

xuhang57 commented 6 years ago

Also, with having cli.py and test/unit/cli.py together (and some other files), It gets an error about import mismatch. See here:https://travis-ci.org/CCI-MOC/hil/jobs/341489660#L3171

Changing ./cli.py to /test_cli.py

xuhang57 commented 6 years ago

So now it is 68%. For unit tests.

xuhang57 commented 6 years ago

screen shot 2018-02-15 at 15 22 39

screen shot 2018-02-15 at 11 49 31

xuhang57 commented 6 years ago

@zenhack done. I forgot to use --cov-append before

zenhack commented 6 years ago

Ok, this looks mostly reasonable to me. Can we preview the results somewhere? you have a screenshot above.

xuhang57 commented 6 years ago

@zenhack if you click the Details on the CI test, you will see two tests, click on coverage/coveralls, you can see the details about the coverage: e.g., why we have a 43% coverage.

screen shot 2018-02-18 at 20 27 46
zenhack commented 6 years ago

Looks like the biggest sources of lacking coverage are:

I'm okay saying fixing #516 is out of scope for this pr, and figuring out a solution separately.

Is it possible to blacklist individual modules for test coverage? It would be nice to have it just not include the hardware drivers in it's accounting, since it's just noise.

xuhang57 commented 6 years ago

Will look into that and get back to you later.

xuhang57 commented 6 years ago

@zenhack , If we separate the tests for hardware drivers from other tests, I think it is very easy to not testing the coverage. Looking into that now.

Also, as for #516, it is saying the test coverage is 0.0%. However, I believe in the current test coverage report, the test coverage is about ~40%. I am not sure whether #516 is still relevant or not.

zenhack commented 6 years ago

Coveralls does seem to report the coverage of that module correctly:

https://coveralls.io/builds/15613016/source?filename=hil/cli.py

So we can probably close #516 once this is merged.

naved001 commented 6 years ago

@naved001 ping. So we should decide what not to test. Any suggestions?

hil/ext/obm/mock.py hil/ext/switches/mock.py

These are the mock drivers we could cover. The CI runs the deployment tests with mock drivers, so I am sure calls to the mock switch will be made.

All the other drivers can be ignored.

xuhang57 commented 6 years ago

@naved001 @zenhack We do not include the test coverage for deployment tests. Should we cover that also?

zenhack commented 6 years ago

Yeah, we should add it to that one too.

xuhang57 commented 6 years ago

So I took a look at the unit tests for obm and switches. I couldn't find any tests with mock drivers. If I am correct, then there is no point for covering them. We should send another PR with adding tests and enable the coverage.

if I am not correct, could you guys please let me know where the tests are? @zenhack @naved001

Thanks!

Update:

nvm. By enabling the deployment tests, mock.py has coverages. Please review.

zenhack commented 6 years ago

I'm happy.

naved001 commented 6 years ago

@jeremyfreudberg are you happy with this PR now? ❌ ➡️ ✔️