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

Add Coveralls GitHub Action #940

Merged
merged 29 commits into from
Jun 28, 2023
Merged

Add Coveralls GitHub Action #940

merged 29 commits into from
Jun 28, 2023

Conversation

michplunkett
Copy link
Collaborator

@michplunkett michplunkett commented Jun 26, 2023

Fixes issue

#914
#937

Description of Changes

This PR creates a Coveralls GitHub action that sends our code coverage to its respective portal and fixes the broken link that was in the README.

Notes for Deployment

Tests and linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.

@michplunkett michplunkett self-assigned this Jun 26, 2023
@michplunkett
Copy link
Collaborator Author

The good news is that our builds are now showing up on Coveralls again and the bad news is that we aren't sending anything meaningful at this point. :(
Screenshot 2023-06-26 at 5 45 28 PM

Altogether though, this means we're like 99% of the way there!

@michplunkett
Copy link
Collaborator Author

Run coverallsapp/github-action@master
  with:
    github-token: ***
    path-to-lcov: OpenOversight/tests/coverage.xml
    coveralls-endpoint: https://coveralls.io/
    debug: false
  env:
    FLASK_APP: OpenOversight.app
Using lcov file: OpenOversight/tests/coverage.xml
Error: ENOENT: no such file or directory, open 'OpenOversight/tests/coverage.xml'

Here's the error I'm ending the day on. I'm gettin' REAL CLOSE.

@abandoned-prototype
Copy link
Collaborator

abandoned-prototype commented Jun 27, 2023

Oops, I wanted to test the stuff in my own branch, but I added my commit here by mistake. Anyways, it looks like with just one small change it works. Great work, thank you!
Verified: Coverage Status

Copy link
Collaborator

@abandoned-prototype abandoned-prototype left a comment

Choose a reason for hiding this comment

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

Feel free to merge if this looks good to you

Makefile Outdated
@@ -48,9 +48,9 @@ populate: create_db ## Build and run containers
.PHONY: test
test: start ## Run tests
if [ -z "$(name)" ]; then \
FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -n auto --dist=loadfile -v OpenOversight/tests/; \
FLASK_ENV=testing docker-compose run --rm web pytest --cov=OpenOversight --cov-report xml:OpenOversight/tests/coverage.xml --doctest-modules -n auto --dist=loadfile -v OpenOversight/tests/; \
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are only used for local testing, but it seems fine to generate coverage reports here as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahhhh, that's right! If we can just have them as printouts for local testing, I think that'd be sufficient. I'll make the small adjustment.

@michplunkett michplunkett marked this pull request as ready for review June 27, 2023 15:43
Comment on lines -51 to +53
FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -n auto --dist=loadfile -v OpenOversight/tests/; \
FLASK_ENV=testing docker-compose run --rm web pytest --cov --doctest-modules -n auto --dist=loadfile -v OpenOversight/tests/; \
else \
FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -v OpenOversight/tests/ -k $(name); \
FLASK_ENV=testing docker-compose run --rm web pytest --cov --doctest-modules -v OpenOversight/tests/ -k $(name); \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we need the coverage files for local testing, but the coverage statistics are helpful to have when running make test.

Comment on lines 5 to 6
- develop
- main
Copy link
Collaborator Author

@michplunkett michplunkett Jun 27, 2023

Choose a reason for hiding this comment

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

The other yml files callout develop and main, so I figured it'd be good for uniformity to do the same within this file as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I reverted this change because we only track coverage on the develop branch.

Comment on lines -7 to +8
- develop
- main
- develop
- main
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Consistent spacing between yml files.

@abandoned-prototype abandoned-prototype merged commit 1a79d1c into develop Jun 28, 2023
3 checks passed
@abandoned-prototype abandoned-prototype deleted the coveralls_fix branch June 28, 2023 03:43
@michplunkett michplunkett linked an issue Jun 30, 2023 that may be closed by this pull request
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.

Fix COVERALLS link
2 participants