gravitee-io / issues

Gravitee.io - API Platform - Issues
64 stars 26 forks source link

[Task] get rid of the 3 flaky test suites in e2e tests #8106

Open a-cordier opened 2 years ago

a-cordier commented 2 years ago

💼 Parent Issue

https://github.com/gravitee-io/issues/issues/{number}

📝 Description

There are 3 flaky test suites that prevent from getting the reports (most of the time)

From what I can remember, the health check test can be fixed by increasing the time between retries

The two others may not be fixed that easily

Note: A describeIfV3 was added to the JWT test in the commit mentioned bellow. This is not a flaky test fix, but there's an actual regression in Jupiter exec. mode that is waiting for a fix.

🔎 Technical Details

see https://github.com/gravitee-io/gravitee-api-management/commit/3b3d9e0a7ae4800f36926c60421093c135e6def5

👽 Additional Information

https://github.com/gravitee-io/gravitee-api-management/pull/2159

Okhelifi commented 2 years ago

It would be nice to add a test on health check to check if endpoint timeout are correctly handled. Following this issue https://github.com/gravitee-io/gravitee-api-management/pull/2454/files

sfrevel commented 1 year ago

I also had a Problem when building the repository in in ADO environment where the timing and the order of tests is totally different. I had to fix the following Tests

For the last one (WebsocketBidirectionalTest) I didn't find any solution than increasing the timeout to 15 seconds. DynamicPropertyUpdaterTest could be fixed make more predictable like this.

  1. remove the CountdownLatch
  2. replace the Assert with the latch with a while loop on future.isDone(). therefore the future(s) - both - could be extracted and the code duplication is also removed

ApiSubscriptionApikeysResourceTest and ApiSubscriptionApikeyResourceTest did even run independently on my local machine! So the whole suite is flaky. The reason for this is a wrong usage of Mockito's reset method - which is an anti pattern. Every test should set up the used mocks for only this test. There should be no re-usage of Mocks. When running the Tests on a different system also the ordering could be different (as in my case with an ADO agent) So the reset() calls are not in the right place and also some of the setups are wrong because the previous tests are different. For a quick fix I had to add

when(permissionService.hasPermission(any(), any(), any(), any())).thenReturn(true);

to the setup method to get the test running independently. But the whole setup with Mockito should be refactored in my opinion.