Extend bucket notification timeout and map to the test timeout #2135
Conversation
williamlardier
commented
Aug 28, 2024
•
williamlardier
commented
- Increase timeout for bucket notification tests, to match the current config (ensure the check timeouts before the step)
- Fixes an issue with kafka cleaner, when an error happens, we have no log and we clear the custer timeout, while the step timeout is -1, resulting in builds never completing (6h+), and improve the timeout.
- Add a custom timeout when checking the DMF volume, as the default 100s is too low, and we have this issue very frequently since months.
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
williamlardier
force-pushed
the
bugfix/ZENKO-4806
branch
from
9cecc0c
to
a2ec22a
Compare
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
|
/create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
An issue where the kafkacleaner never completed was observed in the CI; as a result, the build took 6h before being cancelled. THis fix ensures that we tolerate up to 6min. Note that the current value is 30s * (1000 + 5000) so 3 minutes. Issue: ZENKO-4806
In some cases, the platform will take some time to process the event, so the test will fail, although there is no error. This change also add a sleep between two checks, to reduce the impact on the CPU when multiple tests are running this step in parallel. Issue: ZENKO-4806
Issue: ZENKO-4806
tests/ctst/common/common.ts
Outdated
| @@ -237,14 +237,15 @@ Then('i {string} be able to add user metadata to object {string}', | |||
| } | |||
| }); | |||
|
|
|||
| Then('kafka consumed messages should not take too much place on disk', { timeout: -1 }, | |||
| const kafkaCleanerTimeout = 1800000; // 30min | |||
There was a problem hiding this comment.
30 minutes seems verrrry long for kafka cleaner, which is setup to run every 30 second (or maybe 1-2 minutes) in CI...
There was a problem hiding this comment.
It's already 30min today, this was the config where we got the best results
There was a problem hiding this comment.
I am trying this now 0125893
| @@ -307,6 +308,9 @@ Then('kafka consumed messages should not take too much place on disk', { timeout | |||
|
|
|||
| // If a topic remains in this array, it means it has not been cleaned | |||
| assert(topics.length === 0, `Topics ${topics.join(', ')} still have not been cleaned`); | |||
| } catch (err: unknown) { | |||
There was a problem hiding this comment.
is this not done in cucumber/ctst?
should it not be?
There was a problem hiding this comment.
We have a "finally" that clears the timeout even if we got an error, which mean we never stop the test as its timeout was -1
There was a problem hiding this comment.
There was a problem hiding this comment.
what I mean is that (even with the catch) the exception should be propagated: and eventually reach cucumber/ctst where it shoudl be caught.... and immediately fail the test (even with no timeout)
There was a problem hiding this comment.
Maybe the timeout was not due to an error... At least this ensures we log the error with a message
Errors not catched by the tests are sent to cucumber directly, it'll mark the test as failed. But in the build above, it didn't work
