Open dhruvkb opened 2 years ago
integration tests should not access anything other than the API endpoints. They cannot check ES or the databases etc.
I agree with this sentiment in principle but I think it will conflict with other requirements like https://github.com/WordPress/openverse/issues/684
I suggested elsewhere that someone actually take a step back and write an API unit testing RFC. I can't find where I wrote about it right now but I wonder what the benefit of our current approach is with sample data in flat CSV files that are hand maintained.
We currently hard code IDs into our integration tests. That seems... odd to me. I've never seen that kind of broad approach to testing be that stable. Indeed, over the last couple weeks, the integration tests became increasingly unstable and difficult to manage as upstream instabilities unexpected arose.
I can see the value in a test suite that actually exercises all the actual network requests, especially if we want to rely on it to catch things when upstream responses change unexpectedly. I don't think those need to be the "runs on ever PR" validation tests that they are now. It seems rude to rely on upstream providers in this way when unit tests can run many times a day in rapid succession, locally and in CI. It also makes maintaining the API tests are chore and as someone who spent the last week trying to fix test stability in the frontend and the API, it's really not something I'd like to continue spending time on. Other folks have also had to spend time debugging unstable tests. It's rarely anyone's favourite thing to work on.
Can we take a step back on this and testing as a whole in the API and write an RFC instead of jumping into implementation on this? Right now I fear that the approach we have is fragmented and lacks a clear guiding light/principle by which we make design decisions.
Hi @dhruvkb . I would like to work on this issue. Please assign this to me and give me a head start
@sourav25998 this is currently blocked by the RFC request. Could you pick a different issue to work on? Thanks!
Problem
In merged PR WordPress/openverse-api#844, we had to remove support for running the API tests on the host because the difference in port numbers inside the container and outside was causing only a subset of tests to pass.
Description
The integration tests and unit tests for the API server must run in two different contexts. Currently they are both executed in the same Pytest run. The goal is to separate them into two environments
Additional context
This issue stems from the comment https://github.com/WordPress/openverse-api/pull/844#discussion_r937849145 on merged PR WordPress/openverse-api#844.
Implementation