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

Address Coveralls bug #1100

Merged
merged 21 commits into from
Jun 4, 2024
Merged

Address Coveralls bug #1100

merged 21 commits into from
Jun 4, 2024

Conversation

michplunkett
Copy link
Collaborator

@michplunkett michplunkett commented May 22, 2024

Fixes issue

#1099

Description of Changes

Fixing the Coveralls upload bug.

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.
Screenshot 2024-06-03 at 4 51 04 PM

@michplunkett michplunkett linked an issue May 22, 2024 that may be closed by this pull request
@michplunkett michplunkett self-assigned this May 22, 2024
@michplunkett
Copy link
Collaborator Author

michplunkett commented Jun 2, 2024

Could you take a look at this build, @sea-kelp, and tell me if anything looks obviously wrong: https://github.com/lucyparsons/OpenOversight/actions/runs/9335574698/job/25695125327?pr=1100

It's linked to this PR: #1082

I'm also seeing that the branches have not been updated on here since April 17th: https://coveralls.io/github/lucyparsons/OpenOversight

@sea-kelp
Copy link
Collaborator

sea-kelp commented Jun 3, 2024

@michplunkett fixed - I think what happened is that the coverage file was being created by docker when setting up the bind mount, which gave it weird permissions.

I haven't dug into the linked PR so not sure why that change in particular broke this workflow

Can you check that the coverage has been uploaded successfully?

@michplunkett
Copy link
Collaborator Author

I'll take a look! I tried doing that from the Docker side, but it didn't seem to work: f19fc87

Good find, @sea-kelp! I'll check it out.

@michplunkett
Copy link
Collaborator Author

Looks good to me!

Screenshot 2024-06-03 at 4 06 02 PM

https://coveralls.io/builds/67861750

@michplunkett michplunkett marked this pull request as ready for review June 3, 2024 21:07
@sea-kelp
Copy link
Collaborator

sea-kelp commented Jun 3, 2024

I'll take a look! I tried doing that from the Docker side, but it didn't seem to work: f19fc87

Good find, @sea-kelp! I'll check it out.

The file is mounted from the host machine onto the container filesystem, so the file would need to be created on the host side
https://docs.docker.com/storage/bind-mounts/

@michplunkett michplunkett merged commit 5e0e0d7 into develop Jun 4, 2024
3 checks passed
@michplunkett michplunkett deleted the fix_test_coverage_error branch June 4, 2024 03:51
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.

Address issue with coverage file not uploading
2 participants