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: test_no_permissions() should leave a logs_and_checkpoints/ #487

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

SamuelLarkin
Copy link
Collaborator

@SamuelLarkin SamuelLarkin commented Jun 25, 2024

PR Goal?

Make sure test_no_permissions() doesn't leave a logs_and_checkpoints/ behind.

Fixes?

fixes: #483

Feedback sought?

Approval to merge.

Priority?

low.

Tests added?

None.

How to test?

python -m unittest everyvoice.tests.test_preprocessing.PreprocessingTest.test_no_permissions
find -name logs_and_checkpoints

Confidence?

Good.

Version change?

No.

Related PRs?

None.

Copy link
Contributor

CLI load time: 0:00.24
Pull Request HEAD: f1e9d1e7d3b4f7e07959d6ad96575cba7738895b
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.98%. Comparing base (c928f9b) to head (f1e9d1e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #487   +/-   ##
=======================================
  Coverage   73.98%   73.98%           
=======================================
  Files          45       45           
  Lines        2868     2868           
  Branches      445      445           
=======================================
  Hits         2122     2122           
  Misses        661      661           
  Partials       85       85           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamuelLarkin SamuelLarkin changed the title [WIP] fix: test_no_permissions() should leave a logs_and_checkpoints/ fix: test_no_permissions() should leave a logs_and_checkpoints/ Jun 25, 2024
@SamuelLarkin
Copy link
Collaborator Author

I think we need a more automatic solution where we don't need, for every testcase that write a config, use implicitly with init_context().

Copy link
Member

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

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

I agree we should have a more automatic solution - I'm still a little bit confused why the folder is being created in the first place, but this does appear to fix it - thanks Sam!

@roedoejet roedoejet merged commit 178fedb into main Jun 25, 2024
4 checks passed
@roedoejet roedoejet deleted the dev.sl/483_logs_and_checkpoints branch June 25, 2024 16:29
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.

logs_and_checkpoints folder is erroneously created
2 participants