Skip to content
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 codecov inconsistencies #837

Merged
merged 11 commits into from Jul 17, 2019
Merged

Fix codecov inconsistencies #837

merged 11 commits into from Jul 17, 2019

Conversation

kaustubh-nair
Copy link
Member

Fixes #810

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/mapknitter-reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@kaustubh-nair kaustubh-nair changed the title Start simplecov in test helper Fix simplecov inconsistencies Jul 11, 2019
@codeclimate
Copy link

codeclimate bot commented Jul 11, 2019

Code Climate has analyzed commit 0681e2e and detected 0 issues on this pull request.

View more on Code Climate.

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #837 into main will increase coverage by 14.36%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #837       +/-   ##
===========================================
+ Coverage   58.04%   72.41%   +14.36%     
===========================================
  Files          35       35               
  Lines        1423     1334       -89     
===========================================
+ Hits          826      966      +140     
+ Misses        597      368      -229
Impacted Files Coverage Δ
app/models/map.rb 93.1% <0%> (+2.06%) ⬆️
app/models/warpable.rb 87.5% <0%> (+7.95%) ⬆️
app/helpers/application_helper.rb 50% <0%> (+14.7%) ⬆️
app/controllers/sessions_controller.rb 32.91% <0%> (+32.91%) ⬆️
app/controllers/maps_controller.rb 91.66% <0%> (+91.66%) ⬆️

@kaustubh-nair
Copy link
Member Author

I guess that worked?

@kaustubh-nair
Copy link
Member Author

Let's try one last time

@kaustubh-nair
Copy link
Member Author

Nice!
@jywarren I've think got it fixed
I ran the tests thrice with no decrease in coverage. Let's see how this goes

.travis.yml Outdated
@@ -19,6 +19,7 @@ install:
- bundle check --path vendor/.bundle/ || bundle install --path vendor/.bundle/
- yarn check || yarn install
- bundle exec rake db:setup || bundle exec rake db:migrate
- rm -rf test/reports
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to add this additional step to Travis, causing the build to be slower, I think you could just add test/reports to .gitignore (if its not there already), then remove this folder if its commited. But let's hear from @jywarren.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already in .gitignore
@alaxalves Can you help removing it from git tracking?

@alaxalves
Copy link
Member

I don't think this has worked :( check out codecov @kaustubh-nair

@kaustubh-nair
Copy link
Member Author

Huhhhhhh

@kaustubh-nair kaustubh-nair changed the title Fix simplecov inconsistencies Fix codecov inconsistencies Jul 13, 2019
@kaustubh-nair
Copy link
Member Author

Okay I guess this fixed it? I'm still not sure what the exact problem is. I don't know how this is possible but is travis caching any old reports maybe? @alaxalves?
Anyway @jywarren let's merge this and see how it goes
Thanks!

@alaxalves
Copy link
Member

Okay I guess this fixed it? I'm still not sure what the exact problem is. I don't know how this is possible but is travis caching any old reports maybe? @alaxalves?
Anyway @jywarren let's merge this and see how it goes
Thanks!

Travis doesn't cache old reports because basically we "tell" him what to cache and since we have deleted the reports from git tracking it won't cache. Also everytime Travis uses a different runner and caching is made through a generated .tar.gz pack. But this seems to be working now. Maybe we had to explicitly set the formatter to be codecov.

@jywarren
Copy link
Member

Ok cool! Great work piecing this together. Did you want to merge this now? 🎉👍🏽

@kaustubh-nair
Copy link
Member Author

@jywarren Yes lets merge this now

@jywarren jywarren merged commit d2aea02 into main Jul 17, 2019
@jywarren
Copy link
Member

👍

chen-robert pushed a commit to chen-robert/mapknitter that referenced this pull request Dec 5, 2019
* start simplecov in test helper

* Remove old reports before running

* turn on project

* Removing reports dir after job success

* Update .travis.yml

* Update .travis.yml

* Change formatter declaration location

* Remove reports during build

* Update .travis.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inconsistencies with CodeCov rejections
3 participants