-
Notifications
You must be signed in to change notification settings - Fork 19
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
Fix coverage broken after setup merge #62
Fix coverage broken after setup merge #62
Conversation
@@ -13,7 +13,11 @@ install: | |||
script: | |||
- cd test | |||
- pip freeze # so to help eventual debug: know what exact versions are in use can be rather useful. | |||
- nosetests -xv --process-restartworker --processes=1 --process-timeout=999999999 --with-cov --cov=alignak | |||
- which nosetests | |||
- nosetests -V |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to keep those lines? IMO we sould put that in a script because it is basically 5 lines giving only info about the system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be put in setup_test.sh but what's wrong with having it in travis ? it's really with travis that I wanted to be sure of the versions used.. so my add in .travis file :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well readibility purpose IMO. Add some which travis and so is As ugly as letting print statement in python or letty -x in bash.
Squash Squash |
alignak should somehow be installed for running the tests, so it should be directly importable and this hack must not be necessary.
cca7d9e
to
943e100
Compare
not squashed, it's really better to have few little commits than one bigger, when the little commits are not "do-undo-redo-etc.." of the same thing/change(s) but effectively different things/changes. |
what I'm not sure is to keep/postpone the last commit : "try multiprocess for nosetests" because I know there are times when this apparently triggers some timerace problems.. though not always.. |
but on the other side it really speed up the travis build, up to 2x factor (travis build runners seem to always have 2 cpu available), so if the "timerace" problems I've already encountered doesn't happen more frequently than say once for 4 then it's still a good win .. and actually I'm pretty sure it's the case.. |
Last 3 commits (943e100 1d5fe3e 2e23caa) are "drafts" because you actually do and undo stuff
What is broken?
What kind of issue? |
the kind you've encountered where you have to restart a blocked travis build :p |
943e100
to
0235e84
Compare
+ fix: pip install -e : required for coverage to correctly detect the source files actually used. + update to nose==1.3.7 nose==1.3.7 has some fix regarding a problem with chdir(), which occurs during Daemon startup/daemonize. + clean: use a sane process-timeout for nosetests. Conflicts: .travis.yml
0235e84
to
d89392a
Compare
squashed a bit and removed the "which coverage and nosetests" + their versions. |
Here we go! |
Enh: Tests - Fix coverage and remove old import alignak
No description provided.