search

Kitware / minerva

Minerva: client/server/services for analysis and visualization
Apache License 2.0
36 stars 14 forks source link

Add a real javascript test and enable test coverage #321

Closed jbeezley closed 8 years ago

jbeezley commented 8 years ago

Currently we stand at about 69% coverage in python and 23% in javascript.

One problem with what I did here is that it relies on a particularly ugly setTimeout when loading the test spec. Getting into the details of the jasmine test runner with blanket instrumentation is unpleasant, so I gave up on trying to fix it for real. I'm hoping we can replace the test runner once we get to Girder 2.0, and the issue will go away by itself.

Further limitations on client side testing include that we can't test anything involving uploading files unless we update PhantomJS. I tried but I hit another problem with the Jasmine runner that may or may not be related to the problem I encountered here.

Another nice thing to have that I can do later is a build matrix like I did for girder in https://github.com/girder/girder/pull/1260. For example, we could separate tests involving spark into a separate container so we can get results back quicker for the majority of tests that don't use spark. This will also be necessary to properly report javascript coverage to CDash as well.

mgrauer commented 8 years ago

Up next.

mgrauer commented 8 years ago

We failed the JS coverage test because JS was added in a PR that was merged in here. I'll drop the coverage threshold from 23 to 22% as part of QA.

It might be nice to increase this threshold as a requirement for release 0.1.0, though I feel like I'd be setting a completely arbitrary value. How does 33% sound? 50%?

jbeezley commented 8 years ago

Any threshold at this point will be arbitrary. 33% is going to be pretty easy to hit. 50% is probably better in terms of setting goals.

mgrauer commented 8 years ago

@jbeezley Is there a way to see line by line coverage in the JS files?

jbeezley commented 8 years ago

At the moment, not really. I'll go ahead an set up the build matrix to submit javascript coverage to cdash.

jbeezley commented 8 years ago

@mgrauer http://my.cdash.org/viewCoverage.php?buildid=919332

mgrauer commented 8 years ago

This is great, thanks for the heavy lift. LGTM.

We are still spending ~4 minutes installing pip in travis. I had used travis' caching of pip before, but occasionally ran into trouble that might have cost me all the time I saved by caching. I'm wondering if there are ways to speed up the client side tests without incurring other penalties? Maybe a pip-client-side-tests-freeze file?

jbeezley commented 8 years ago

The primary problem seems to be recompiling numpy. For some reason, caching the binary wheel isn't working. I'm not sure if it is because numpy disables it, or we are doing something wrong. We could do one of the following to speed it up:

mgrauer commented 8 years ago

Weird. We are caching the virtual env dir, or at least trying to.

At first I thought we weren't caching this anymore, but I guess it just isn't working any longer.

jbeezley commented 8 years ago

Is that the default virtualenv that travis uses? We aren't sourcing $HOME/virtualenv/python2.7.9/bin/activate anywhere. If so, it's possible travis is stepping on the caching that you are trying to do for it's own purposes.

mgrauer commented 8 years ago

Travis is calling source ~/virtualenv/python2.7/bin/activate for us, we don't need to do it.

I roughly found the point where it split.

The Minerva Fontello PR has a PR build with pip 7.1.2 where InstallPythonRequirements takes ~27 seconds.

A few PRs later, Remove gaia dependency PR has a PR build with pip 6.0.7 where InstallPythonRequirements takes ~210 seconds, and my commit message indicates I cleaned the Travis cache.

Both of these source the same virtualenv path.

Seems like we've been with pip 6.0.7 ever since. Maybe some previous commit had upgraded pip, and then this got cached, but then we stopped upgrading pip in travis, and once we cleaned the cache we are on pip 6.0.7, which is breaking our usage of the cached virtualenv?

This has re-convinced me that caching is hard. We should probably stop caching travis pip altogether for clarity as we aren't using it, or commit to pip caching and see if upgrading pip in our .travis.yml helps.

@jbeezley you recently made changes in girder related to setuptools entry_point for plugins. Does that expose some path we can take advantage of in Minerva which relates to this? i.e. if we are making changes soon on our python packages install we may not want to solve this caching problem now.

jbeezley commented 8 years ago

If we are using pip 6.0, that would be the reason caching doesn't work because the wheel caching wasn't added until 7.0.

RE entry_point: It is only currently relevant for server-side only plugins.