-
Notifications
You must be signed in to change notification settings - Fork 54
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 most tests for windows computers #467
Conversation
Miauwkeru
commented
Dec 1, 2023
- Check whether effective_ids is enabled for the system
- Use type consistently
- Open files as UTF-8 by default
- Fix paths in test_target.py
- Change all occurrences to flow.record path to self.target.fs.path
- Fix issues regarding the resolver in trusteddocs
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #467 +/- ##
==========================================
+ Coverage 73.88% 73.94% +0.06%
==========================================
Files 259 259
Lines 20901 20878 -23
==========================================
- Hits 15442 15439 -3
+ Misses 5459 5439 -20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Accidentally added or intentional addition? Missing a __init__.py
if the latter case.
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.
Oh, it was an accident on my part that I added the nixos/_os.py
in this pr... I will take it with me to a future iteration
@@ -73,7 +73,7 @@ def trusteddocs(self) -> Iterator[TrustedDocumentsRecord]: | |||
ts=key.ts, | |||
type=value.type, | |||
application=application, | |||
document_path=self.target.resolve(value.name), | |||
document_path=self.target.fs.path(self.target.resolve(value.name)), |
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.
Perhaps .resolve()
should return a path?
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.
That might be a good idea for the future yes.
1d1c567
to
589ff69
Compare
6d5e97c
to
ea0f030
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.
LGTM
Made the ticket and resolved the review comments
* Check whether effective_ids is enabled for the system * Use __type__ consistently * Open files as UTF-8 by default * Fix paths in test_target.py * Change all occurrences to flow.record path to self.target.fs.path * Fix issues regarding the resolver in trusteddocs
* Check whether effective_ids is enabled for the system * Use __type__ consistently * Open files as UTF-8 by default * Fix paths in test_target.py * Change all occurrences to flow.record path to self.target.fs.path * Fix issues regarding the resolver in trusteddocs
* Check whether effective_ids is enabled for the system * Use __type__ consistently * Open files as UTF-8 by default * Fix paths in test_target.py * Change all occurrences to flow.record path to self.target.fs.path * Fix issues regarding the resolver in trusteddocs