test: add unit tests for content parsing and input reading #17
Conversation
- Add tests for `copyToClipboard` function to verify clipboard operations. - Add tests for `readFromStdin` function to ensure proper stdin reading. - Add tests for `readFromFile` function to verify file reading functionality. - Add tests for `parseContent` function to check parsing of direct text input, stdin, and file inputs. These tests enhance the robustness of the application by ensuring all core functionalities are working as expected.
supitsdu
added
the
testing
|
I love you added tests |
Thanks a ton for your help with parseContent! Glad you love the new tests. |
| } | ||
|
|
||
| // Helper function to create temporary file for testing | ||
| func createTempFile(content string) *os.File { |
There was a problem hiding this comment.
Here is a safer and cleaner way to do
| func createTempFile(content string) *os.File { | |
| func createTempFile(t *testing.T, content string) *os.File { | |
| file, err := os.CreateTemp("", "testfile") | |
| if err != nil { | |
| t.Fail("creating temporary test file") | |
| } | |
| // register the cleaner without having a need to handle the file removal with a defer that can be forgotten | |
| t.Cleanup (func() { | |
| defer os.Remove(file.Name()) | |
| }) | |
| file.WriteString(content) | |
| file.Close() | |
| return file | |
| } | |
If you have a look at os.CreateTemp code
https://cs.opensource.google/go/go/+/go1.22.4:src/os/tempfile.go;l=33
You will see the code is using tempdir when using "" as empty folder.
So you are creating a folder, but nothing delete it when the test are over.
Then, now you have a better understanding, there is the better way to write it
| func createTempFile(content string) *os.File { | |
| func createTempFile(t *testing.T, content string) *os.File { | |
| file, err := os.CreateTemp(t.TempDir(), "testfile") | |
| if err != nil { | |
| t.Fail("creating temporary test file", err) | |
| } | |
| file.WriteString(content) | |
| file.Close() | |
| return file | |
| } |
Please have look at https://pkg.go.dev/testing#T.TempDir
and its implementation https://cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/testing/testing.go;l=1229
No need for cleaning up the file, then because the whole temp folder is nuked by the cleanup added via t.TempDir
There was a problem hiding this comment.
Here is a safer and cleaner way to do
I opened an issue in an existing linter to report the use case I discovered here
|
|
||
| func TestParseContentStdin(t *testing.T) { | ||
| stdinInput := "Stdin input\n" | ||
| r, w, _ := os.Pipe() |
There was a problem hiding this comment.
The very same way you created createTempFile, could do this
| r, w, _ := os.Pipe() | |
| function replaceArgs(t *testing.T) (r *os.File, w *os.File) | |
| r, w, err := os.Pipe() | |
| if err != nil { | |
| t.Fail(err) | |
| } | |
| originalStdin := os.Stdin | |
| t.Cleanup (func() { | |
| os.Stdin = originalStdin | |
| r.Close() | |
| w.Close() | |
| }) | |
| os.Stdin = r |
This way you can reuse it safely everywhere
There was a problem hiding this comment.
I asked for code review on slack.
Someone suggested using testscript
Someone made a proof of concept.
It allows to do not overload os.Std*
https://github.com/datacharmer/clipper/blob/add-testscript/cli%2Fclipper_test.go
There was a problem hiding this comment.
Thank you @datacharmer, BTW
|
|
||
| func TestParseContentFromFile(t *testing.T) { | ||
| contentFromFile := "Content from file" | ||
| fakeFile := createTempFile(contentFromFile) |
There was a problem hiding this comment.
| fakeFile := createTempFile(contentFromFile) | |
| fakeFile := createTempFile(t, contentFromFile) |
Please have a look at #17 (comment)
Then the defer can be removed
There was a problem hiding this comment.
Always nice to have feedbacks like these, constructive and straightforward. This reviews seems to add a proper resource cleanup and error handling, making the test suite safer and easier to maintain. Loved it!
There was a problem hiding this comment.
You are welcome, I'm glad you like my feedbacks
|
For records, archive, and a better understanding, this PR adds tests for methods added with #16 |
copyToClipboardfunction to verify clipboard operations.readFromStdinfunction to ensure proper stdin reading.readFromFilefunction to verify file reading functionality.parseContentfunction to check parsing of direct text input, stdin, and file inputs.These tests enhance the robustness of the application by ensuring all core functionalities are working as expected.