-
Notifications
You must be signed in to change notification settings - Fork 3k
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
SimpleCov Coverage Report #705
SimpleCov Coverage Report #705
Conversation
71f23b9
to
5a333c0
Compare
5a333c0
to
42ffc6f
Compare
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.
@@ -51,6 +51,8 @@ group :development, :test do | |||
gem "debug", platforms: %i[ mri windows ] | |||
gem "brakeman", require: false | |||
gem "rubocop-rails-omakase", require: false | |||
gem "simplecov", require: false | |||
gem "dotenv-rails" |
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.
I think simplecov
should only be in the :test
group right?
Also, I recently moved dotenv-rails
to the :development
group only to avoid unexpected, failing tests when a dev has their .env
setup incompatible with the test expectations. I think this will still work if we pass the ENV to the test command right?
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.
Yes, this should be in the test group and it works if we pass the env from the CMD. Working on changes
@harrrykp also, for future PRs, would you mind configuring your Github email (in settings) to match your git client email? It makes it a little easier to review with a single user in the commit history. |
See: simplecov-ruby/simplecov#718 Looks like Rails test parallelization is causing coverage to drop: parallelize(workers: :number_of_processors) We should be seeing ~85% coverage on this codebase. |
Ohh, got it. Will be reviewing this issue for a possible solution. |
@harrrykp closing this out for now. If you'd like to continue work on it just let me know and I'll re-open! |
Description
fixes #699