-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Remove coveralls, add codecov #5954
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5954 +/- ##
==========================================
+ Coverage 74.67% 80.75% +6.08%
==========================================
Files 89 97 +8
Lines 5402 5561 +159
==========================================
+ Hits 4034 4491 +457
+ Misses 1368 1070 -298
Continue to review full report at Codecov.
|
@@ -20,8 +20,6 @@ namespace :test do | |||
|
|||
desc "Run rails and teaspoon tests" | |||
task :all => :environment do | |||
require 'coveralls/rake/task' | |||
Coveralls::RakeTask.new | |||
if ENV['GENERATE_REPORT'] == 'true' | |||
require 'ci/reporter/rake/test_unit' | |||
Rake::Task["ci:setup:testunit"].execute |
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.
@jywarren need some info on these tasks
.travis.yml
Outdated
# - if [ $TASK != "rake test:all" ]; then | ||
# echo -e '<?xml version="1.0" encoding="UTF-8"?>' > output.xml; | ||
# tail -n +2 -q ./test/reports/TEST*.xml >> output.xml; | ||
# fi |
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.
@jywarren @publiclab/reviewers What are these lines supposed to be doing? I'm not sure if they are related to coveralls or not
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.
They're related to Coveralls, they generate the XML to be pushed to Coveralls. You can remove those.
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.
Not sure about this. Dangerbot uses the xml too I think.
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.
These were intended to search through standard formatted JUnit test reports and copy them into the Dangerbot comment in the PR. They broke some time ago, but were a really nice way to help people see where errors were occurring. I'd like to leave them in so we can fix them again some day!
I've got codecov setup but the build is failing, because of some errors in the Rakefile I think |
Ah, i think you'll have to remove
Probably in the rakefile, line 29? |
Not sure why project check is turning up even though it's off. I guess it should be fine after merging? |
@jywarren On second thought, let's not merge this till I fix the inconsistencies on mapknitter. I might have to change the reporter |
@publiclab/reviewers Any idea why a unit test is failing? I haven't made changes to the code. That's weird. 😕 |
For sure, the error is not related to your code. I have restarted the job. |
We're facing the same coverage drop here too :/ |
Is the coverage drop consistent? I.e. if it's not related to code, are we seeing a non-deterministic behavior, perhaps resulting from the order of completion of tests, or from other branch tests getting mixed up somehow? Or is it showing the same exact drop each time this is run? Can you open an issue on CodeCov's tracker and link between here and there to try to get some help on this? Thank you! |
* typo replace omment with comment * Update blog.css * Update .travis.yml * Update .travis.yml * add codecov branch to travis for debugging
Let's see how this does and then we can resolve conflicts and merge! |
@Uzay-G can you see why the tests didn't pass here? https://travis-ci.org/publiclab/plots2/builds/630645697?utm_source=github_status&utm_medium=notification I restarted it after resolving conflicts but not sure why it had failed before. I hope it doesn't this time! |
I looked at it but don't understand why it bugged out then and worked now although nothing was changed. The error just says |
Wow! This would increase our coverage to 80% then... This would be awesome. Do you think it's ready to merge then? From the chatroom:
|
Let's do this!!! |
🎉 🎉 🎉 🎉 🎉 |
Nice! Yeah I think it's ready and it should work! 🎈 👍 |
Ah! I don't understand why but for some reason the codecov bot is not commenting on new pull requests. This issue is really a mystery... |
Ok actually it worked but there is a problem. See here for what it did: #7086. It returns the error: exclamation No coverage uploaded for pull request base (master@e16e4bc). |
Ah, it's supposed to compare to the base that is the HEAD position of the
master branch, not a static version, right?
…On Thu, Jan 2, 2020 at 1:19 PM Uzay-G ***@***.***> wrote:
This is the error:
[image: codecov]
<https://user-images.githubusercontent.com/52892257/71683845-f4e03c00-2d93-11ea-8a2b-01cbf4d7cfa4.png>
I think we need to edit the settings on the codecov dashboard so that the
base is more recent maybe??
I'm not sure because I don't have permission to see the settings
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5954?email_source=notifications&email_token=AAAF6J5JED24A5PJ5R6JR4LQ3YVH3A5CNFSM4H3ZXL52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH67S4Q#issuecomment-570292594>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF6J3AMMYGR4DIDWYICALQ3YVH3ANCNFSM4H3ZXL5Q>
.
|
Yeah it needs to compare to the base. What is it comparing to right now? |
I don't know, but is it potentially an issue that if tests haven't changed
at all, then the PR doesn't meet the minimum requirement?
…On Thu, Jan 2, 2020 at 1:33 PM Uzay-G ***@***.***> wrote:
Yeah it needs to compare to the base. What is it comparing to right now?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5954?email_source=notifications&email_token=AAAF6J3Q6PSWLUT7W2MA4F3Q3YW45A5CNFSM4H3ZXL52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH7AWRI#issuecomment-570297157>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF6J6DFKUESFOMEUSYEA3Q3YW45ANCNFSM4H3ZXL5Q>
.
|
I don't think it's because of the pull request changes because all of the pull requests have the same message. The problem is that it is comparing to a commit that doesn't have a codecov report so it can't compare the changes. |
This page might help us: https://docs.codecov.io/docs/comparing-commits |
I think I know the problem and will try to fix it when I come back |
OK, awesome!
…On Thu, Jan 2, 2020 at 2:11 PM Uzay-G ***@***.***> wrote:
I think I know the problem and will try to fix it when I come back
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5954?email_source=notifications&email_token=AAAF6J5RZ63G2XTY47ZQDZDQ3Y35XA5CNFSM4H3ZXL52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH7D4KY#issuecomment-570310187>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF6J7TXKCIKYNLBLQ7MVTQ3Y35XANCNFSM4H3ZXL5Q>
.
|
Hi @Uzay-G (ref #5954) this is a good example PR: #6957
Does this mean that as soon as we merge another PR to master, it'll have a recorded coverage base for that commit? OH! You know what? What if squashing and merging is causing a new commit hash to be generated, and so we have to be sure tests are run on that too so that it files a coverage report for the newly generated squashed commit??? |
Yeah maybe we need to merge one to have a base with a coverage report. I'm not sure what you mean by the second part bur merging one might be a good idea. |
We could merge #7076 to try it out |
OK i merged #7089 Let's look again. If it doesn't work, we can try merging WITHOUT squashing to test if that does not generate a different commit and so the coverage would be saved??? |
It worked here: #5724 (comment). I think the issue is resolved! |
Since you merged the pr, codecov is working. Yay we fixed it! 🎂 |
Ah! I think one complicating factor is that we are seeing it update
progressively as the tests run...! We may just need to wait a bit longer.
…On Thu, Jan 2, 2020 at 5:57 PM Uzay-G ***@***.***> wrote:
Since you merged the pr, codecov is working. Yay we fixed it! 🎂
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5954?email_source=notifications&email_token=AAAF6J6OQJ6F7WEEGRPTAP3Q3ZWONA5CNFSM4H3ZXL52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH7W5XA#issuecomment-570388188>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF6J5BGR5ESG5VSKT67PTQ3ZWONANCNFSM4H3ZXL5Q>
.
|
Yeah I agree |
* Remove coveralls, add codecov * Add env variable * Comment reporting lines * Change env variable * Remove coveralls from rake file * Fix travis and rakefile * Remove project for codecov * Remove old reports before running * typo replace omment with comment (publiclab#7042) * typo replace omment with comment * Update blog.css * Update .travis.yml * Update .travis.yml * add codecov branch to travis for debugging Co-authored-by: Uzay-G <[email protected]> Co-authored-by: Jeffrey Warren <[email protected]>
* Remove coveralls, add codecov * Add env variable * Comment reporting lines * Change env variable * Remove coveralls from rake file * Fix travis and rakefile * Remove project for codecov * Remove old reports before running * typo replace omment with comment (publiclab#7042) * typo replace omment with comment * Update blog.css * Update .travis.yml * Update .travis.yml * add codecov branch to travis for debugging Co-authored-by: Uzay-G <[email protected]> Co-authored-by: Jeffrey Warren <[email protected]>
Partially fixes #5931