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

Close temporary file after writing parsed test results #67

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

hamir-suspect
Copy link
Contributor

@hamir-suspect hamir-suspect commented Aug 21, 2023

We are opening the same file multiple times:

tmpFile, err := os.CreateTemp(dirPath, "result-*.json")
			if err != nil {
				return err
			}

and then again in the cli.WriteToFile: (here)

			_, err = cli.WriteToFile(jsonData, tmpFile.Name())
			if err != nil {
				return err
			}

Since we are looping over paths we need to close the file explicitly and not using defer statement.

@hamir-suspect hamir-suspect requested a review from skipi August 21, 2023 14:29
@ralopes
Copy link
Contributor

ralopes commented Aug 21, 2023

@hamir-suspect good catch, but I think we also should fix the creating the file twice.

What if we create a WriteFile function which receives an *os.File (and not explicitly closes it), so we let the caller handle opening and closing the file.

func WriteFile(data []byte, file *os.File) (string, error) {
	return writeToFile(data, file)
}

@hamir-suspect hamir-suspect requested a review from ralopes August 22, 2023 08:48
@hamir-suspect hamir-suspect force-pushed the has/fix-too-may-open-files branch from 742edc6 to 9863d49 Compare August 22, 2023 10:22
@skipi
Copy link
Member

skipi commented Aug 23, 2023

👏🏻 Looks good to me. I'll release a new version of the CLI later this week.

@skipi skipi merged commit e484b8c into master Aug 25, 2023
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.

3 participants