search

elixir-cloud-aai / ga4gh-sdk

Generic SDK and CLI for GA4GH API services
Apache License 2.0
13 stars 2 forks source link

Add testing with other implementations of TES (other than funnel) #15

Open aaravm opened 4 months ago

aaravm commented 4 months ago

This is issue is in reference to https://github.com/elixir-cloud-aai/ga4gh-sdk/pull/13#discussion_r1663364350 Currently, the codebase uses Funnel for unit testing. Since it might not be healthy to tightly couple one specific implementation with a generic library, we should use multiple implementation for testing purposes.

uniqueg commented 3 months ago

I think tests should be added continuously with each code change, not after code was merged. Otherwise it tends not to get done.

For our other projects, we generally make the availability of comprehensive tests a prerequisite for changes to be merged (with very few exceptions for toy/sandbox code), and I think this is a good idea here as well.

Same with documentation, see: See https://github.com/elixir-cloud-aai/ga4gh-sdk/issues/16#issuecomment-2250191721

aaravm commented 3 months ago

Currently, I have already made the unit tests using Funnel. How many other implementations of TES do you think should be used for tests?

I think tests should be added continuously with each code change, not after code was merged. Otherwise it tends not to get done.

For our other projects, we generally make the availability of comprehensive tests a prerequisite for changes to be merged (with very few exceptions for toy/sandbox code), and I think this is a good idea here as well.

Same with documentation, see: See #16 (comment)

uniqueg commented 3 months ago

Currently, I have already made the unit tests using Funnel. How many other implementations of TES do you think should be used for tests?

I think tests should be added continuously with each code change, not after code was merged. Otherwise it tends not to get done. For our other projects, we generally make the availability of comprehensive tests a prerequisite for changes to be merged (with very few exceptions for toy/sandbox code), and I think this is a good idea here as well. Same with documentation, see: See #16 (comment)

None. In fact, for most tests (unit and integration), you don't really need external services at all. Consider mocking responses from external dependencies instead. After all, we want to tese our code, not theirs, and you could easily get failing tests through no fault of yours - especially if the network is involved.

It's fine to keep Funnel for end to end tests. It is of course nice to know that we actually get expected responses for our requests from real services.