-
-
Notifications
You must be signed in to change notification settings - Fork 234
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: broken TestHardcodedIncludeDirectiveDDOS2 on windows #1136
fix: broken TestHardcodedIncludeDirectiveDDOS2 on windows #1136
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1136 +/- ##
==========================================
+ Coverage 82.72% 83.91% +1.19%
==========================================
Files 162 166 +4
Lines 9080 7891 -1189
==========================================
- Hits 7511 6622 -889
+ Misses 1319 1019 -300
Partials 250 250
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
internal/seclang/parser_test.go
Outdated
@@ -169,6 +169,7 @@ func TestHardcodedIncludeDirectiveDDOS2(t *testing.T) { | |||
if err != nil { | |||
t.Fatal(err) | |||
} | |||
tmpFile2.Close() |
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.
shouldn't we assert errors on tmpFile2.Close()?
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.
Done
7b25147
to
03cce15
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.
Thanks! I think it is way easier to create the file on before hand instead of relying on an existing unrelated file in the system. You can do os.CreateTemp(t.TempDir(), "existing.txt")
. This could be merged right away after that.
03cce15
to
87ddf45
Compare
@jcchavezs I am not sure what you mean. I do not want to touch the existing code in this PR, just fix the missing Close statement (which is also required with os.CreateTemp). |
This is a small fix for the TestHardcodedIncludeDirectiveDDOS2 on windows.
Windows refuses to delete files as long as the file descriptor is open. Closing the temporary file fixes this issue and go test is able to cleanup the files.