-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
enable test coverage on coveralls #7215
Conversation
Great, seem to work now, will cleanup/squash the commits. |
Oh, the link : https://coveralls.io/r/ipython/ipython |
Are we able to do any JavaScript coverage testing? |
Great idea by the way. Big fan of coveralls. |
The website seem to lack activity though. Note to other, coverall can post on all PR. I have it deactivated for now. |
Works now, just need to copy the |
Probably but I don't know how with casper/phantom |
Well, I was thinking more like we use their node coverage plans. It would mean that we start to demarcate pieces of our IPython frontend JavaScript as working with node (which I'd like to see anyway, I could use it in Atom in a less hacky way then). |
Yeah, that's one of the reason I fight against jquery and other. I would like that to. |
Ping. |
try: | ||
cov.xml_report(outfile='ipy_coverage.xml') | ||
except CoverageException: | ||
pass |
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.
Probaby not a good idea to fail silently. Warn, at least?
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.
Done.
I'm just unhappy, either coverall has been bought, or hacked, I need to authorize a new key to access the results, which does not look like coveralls for now. And it requires full read/write access. |
@Carreau does that mean we should abandon this? |
No, probably not. I think it's just a mistake of coveralls that push a testing key from the company that made the website. I've tweeted a inquery and will contact by mail later if no news or not fixed or not clarified. |
try: | ||
cov.xml_report(outfile='ipy_coverage.xml') | ||
except CoverageException as e: | ||
print('Generating coverage report failed. Are you running javascript tests only?') |
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.
Should print the exception itself as well, yes?
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.
Seem like extra noise on js test, but if you wish so, done.
LGTM
Any update? |
Seem to be fixed. I had to do a bit of browser incognito mode hopping, but it does not require full access anymore. |
enable test coverage on coveralls
In progress, does not work yet:https://coveralls.io/r/Carreau/ipython
I can submit manually the results from localhost, but it seem not to want to work through travis.
Works now.