Conversation
mcarmonaa
left a comment
mcarmonaa
left a comment
There was a problem hiding this comment.
Do you think it would be worthy create a suite for download_test.go with a setup and a teardown for the Helper stuff so the directory structure would be reused across the tests or it would generate conflicts among them?
Anyway apart of the nitpicky comments LGTM! thanks!
downloader/download_test.go
Outdated
| // 1) try to create *siva.NewLibrary using fs with broken OpenFile method | ||
| // <expected> error that contains broken fs mocked error's text | ||
| func TestLibraryCreationFailed(t *testing.T) { | ||
| h, closer, err := testhelper.NewHelper() |
There was a problem hiding this comment.
maybe close instead of closer would fit better for a function
downloader/download_test.go
Outdated
| require.NoError(t, err) | ||
|
|
||
| h.FS = testhelper.NewBrokenFS(h.FS, testhelper.BrokenFSOptions{FailedOpen: true}) | ||
| _, err = h.NewLibrary() |
There was a problem hiding this comment.
The Helper struct already holds an instantiated library, couldn't we use that library for each test rather than instantiate a new one?
There was a problem hiding this comment.
Sometimes we create libraries based on the mockup fs, I did the next changes:
- Refactored all test to run with one suite using one helper
- after each test
closeis invoked - If we do not mock up fs - we will use default library(instantiated in constructor)
| testPrivateRepo = &test{ | ||
| locID: borges.LocationID("2ea758d7c7cbc249acfd6fc4a67f926cae28c10e"), | ||
| repoIDs: []borges.RepositoryID{ | ||
| borges.RepositoryID("github.com/lwsanty/super-private-privacy-keep-out"), |
There was a problem hiding this comment.
cool name for a repository 🤣
1) add some coverage cases from src-d#62 (comment) 2) add fs mockup cases 3) refactor 4) add small documentation closes src-d#63 Signed-off-by: lwsanty <lwsanty@gmail.com>
lwsanty
force-pushed
the
download-tests
branch
from
2297095 to
1dd0fff
Compare
mcarmonaa
left a comment
mcarmonaa
left a comment
There was a problem hiding this comment.
I think it's fine as it is so we don't need to add more dependencies but when we need more complex functionality for suite tests we used to work with https://github.com/stretchr/testify#suite-package
3 participants
closes #63
Signed-off-by: lwsanty lwsanty@gmail.com