ci: collect and upload test coverage to coddecov vec-337

[dwelch-spike]

No description provided.

[codecov-commenter]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[dwelch-spike]

@DomPeliniAerospike this is ready for review, please give it a look when you can.

[DomPeliniAerospike]

Does code coverage need each test category to complete? It shouldn't cause a problem, but it might be nice to generate the report even if a test category fails?

[DomPeliniAerospike]

Did you verify that code coverage will be displayed? The message above implies that code coverage will be available upon merge when a PR to the Dev branch is made. It might be good to verify that this is working as expected once this is merged.

[dwelch-spike]

@DomPeliniAerospike

1. The coverage job fails if any of the test jobs fail. All tests should pass before a PR is merged anyways so this doesn't seem like an issue to me.

2. The coverage is currently available on the codecov website, but it won't be displayed on PRs or the repo until these changes are merged to main. I don't think merging to dev will be enough.

[DomPeliniAerospike]

@DomPeliniAerospike

1. The coverage job fails if any of the test jobs fail. All tests should pass before a PR is merged anyways so this doesn't seem like an issue to me.
2. The coverage is currently available on the codecov website, but it won't be displayed on PRs or the repo until these changes are merged to main. I don't think merging to dev will be enough.

Number 1 is definitely not an issue, adding coverage reports to all tests just makes all the actions more verbose. This is also redundant, since all coverage reports (besides extensive_vector_search tests) will generate the same report. Should be fine for now, but it might be worth removing the redundancies in future pipeline work.

For number 2, I'm not sure how to access the code coverage website for this repository, but I can wait until actions provides a link in a future PR and take your word for it.

[DomPeliniAerospike]

Looks good to me!

[dwelch-spike]

@DomPeliniAerospike

1. I thought the coverage would be different for each test suite. Some test async vs sync, some with and without tls. Won't that hit different code paths?
2. Sent you the link in slack

[DomPeliniAerospike]

@DomPeliniAerospike

1. I thought the coverage would be different for each test suite. Some test async vs sync, some with and without tls. Won't that hit different code paths?
2. Sent you the link in slack

This is a good point. Currently, only async is tested. When I was setting up actions, I changed them to only test aio rather than both to save time. I must've forgot to change it back. In reality, we probably don't need to run sync and async for all tests since they use the same baseClient and channel provider, but running both for all would be the most safe (for now). I committed a changes the action to run both for all tests.

The coverage difference between TLS configurations is slight. MTLS and TLS should have the same coverage, but yes, non-TLS setups will have slightly different coverage the TLS setups.

There might be a better way than running all tests for all setups. Maybe we just need a smaller suite which just connects the client with different setups and ensure database operations can be done.

If we did this, we could avoid running the async and sync suites in entirety for every test, since difference in client setups are accounted for elsewhere. We would need to do a bit of an overhaul actions overhaul, but it we could address this when we begin to finalize the CI/CD pipeline to reduce complexity.

TLDR: Lets do full async and sync suites for each setup, but the test verbosity and complexity could be reduced by making a client connection suite of some sort which will accomplish the same coverage.