Run aio pool tests in docker #467
Conversation
pool/pool_aio_test.go
Outdated
| @@ -1,5 +1,3 @@ | |||
| //go:build aiotest | |||
There was a problem hiding this comment.
does it mean that a dev has to have docker for developing SDK?
There was a problem hiding this comment.
Correct. The same method already exists in the rest gate and works perfectly
There was a problem hiding this comment.
i can't deny this cuz i have docker, but im really 😟 about this
There was a problem hiding this comment.
already exists in the rest gate and works perfectly
sure, i do not think that your code or docker client lib is bad, i just meant that i am not sure we need to force such a requirement on everybody: if i want to make a contribution to a go lib and run tests locally, i would expect me to have go and... all? while CI on the GH side can run everything with all the dependencies we want
what @roman-khimov thinks about it?
There was a problem hiding this comment.
Docker is a very common software, I tend to think almost all backend developers have it on board
There was a problem hiding this comment.
SDK is just a Go lib, i don't expect to require anything but pure Go to maintain it. If we wanna test AIO, lets test it within https://github.com/nspcc-dev/neofs-aio where Docker is natural. Or make docker test an optional feature make test-docker
There was a problem hiding this comment.
Optional tests never really work, it has to be a part of the regular testing routine. I agree that in general it should be possible to cover the code with pure synthetic tests, but at the same time these are valuable as well, they prove that the system works as a whole.
What can be done is:
- a separate target
- but a mandatory GH action always running it
Maybe that's the optimal combination, but this still raises some questions of proper coverage counting.
There was a problem hiding this comment.
lets try to cover both goals. If we decide that Docker requirement can not be avoided, then i dont mind to have it if tests will really help
There was a problem hiding this comment.
I left make test as is and everyone may use it for SDK development. Also I've added a new make target test-with-docker for all tests, including docker AIO. Updated GH workflow to use test-with-docker
|
BTW, my |
8a4e2e8 to
f79cfd1
Compare
5494893 to
68f7ce6
Compare
I've updated the go.mod in a separate commit, but without an explicit |
68f7ce6 to
893c503
Compare
80bba8f to
a091f9e
Compare
|
There is some issue with the latest go versions. Go 1.19.11 and 1.20.6 rise errors. I see two solutions:
Of course, it is for a few days, until the bug is fixed. What do you see? @roman-khimov |
|
I think we can just wait a little before merging this. |
|
BTW, the bug is fixed in testcontainers now and we just need a new release from them. |
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
closes #445 Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
…unction Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
For local development we left `make test` command to check regular unit tests. For GH actions we will use one more step with docker AIO container. Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
a091f9e to
a295c21
Compare
|
Rebased from the latest master version. Updated testcontainers-go to v.0.22.0. Right now all tests are green |
closes #445