Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] tests: autoretry and cursor commit #162990

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Xavier-Do
Copy link
Contributor

This should improve autoretry mecanism overall, and also avoid chain error, even without autoretry, when commiting/rollbacking/closing the cursor opened from a TransactionCase.

@Xavier-Do Xavier-Do changed the base branch from 17.0 to master April 23, 2024 12:33
@Xavier-Do Xavier-Do marked this pull request as draft April 23, 2024 12:33
@robodoo
Copy link
Contributor

robodoo commented Apr 23, 2024

@C3POdoo C3POdoo added the RD research & development, internal work label Apr 23, 2024
@Xavier-Do Xavier-Do marked this pull request as ready for review April 23, 2024 14:17
@C3POdoo C3POdoo requested review from a team, HydrionBurst and ryv-odoo and removed request for a team April 23, 2024 14:20
Copy link
Collaborator

@xmo-odoo xmo-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo override=ci/security

break
if result.had_failure:
_logger.runbot('Disabling auto-retry after a failed test')
os.environ.pop('ODOO_TEST_FAILURE_RETRIES')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's going to raise a KeyError if the envvar is not present, is that intended?

Also why not just lift this info to a global variable / counter if we're going to access it multiple times? That way this branch can just update the global.

@Xavier-Do Xavier-Do force-pushed the master-autoretry-imps-xdo branch 2 times, most recently from e1344db to b9f7307 Compare April 24, 2024 08:45
@Xavier-Do
Copy link
Contributor Author

@robodoo r+

@robodoo
Copy link
Contributor

robodoo commented Apr 25, 2024

@Xavier-Do you may want to rebuild or fix this PR as it has failed CI.

@robodoo
Copy link
Contributor

robodoo commented Apr 25, 2024

@Xavier-Do because this PR has multiple commits, I need to know how to merge it:

  • merge to merge directly, using the PR as merge commit message
  • rebase-merge to rebase and merge, using the PR as merge commit message
  • rebase-ff to rebase and fast-forward

@Xavier-Do
Copy link
Contributor Author

@robodoo r+

@robodoo
Copy link
Contributor

robodoo commented Apr 25, 2024

I'm sorry, @Xavier-Do: this PR is already reviewed, reviewing it again is useless.

@Xavier-Do
Copy link
Contributor Author

@robodoo rebase-ff

@robodoo
Copy link
Contributor

robodoo commented Apr 29, 2024

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Apr 29, 2024
Commiting or rollbacking the cursor during test will lead to failure
in cascade, because the cursor cannot be rollbacked at the end of the
test, and cannot be closed proprely.

In some case, it can even provoque failure on following test, maybe
because some cleanup are not done proprely.

This is also breaking the autoretry mecanism since the cursor cannot be
rollbacked, leading to error not related to the first failure.

Part-of: #162990
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Auto retry is mainly useful to avoid the need to rebuild in case of
random failure, but once one test failed, this mecanism is useless,
creates noises in logs, increase laod by retrying test for no reason,
...

This commit should disable this mecanism after the first error.

closes #162990

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Commiting or rollbacking the cursor during test will lead to failure
in cascade, because the cursor cannot be rollbacked at the end of the
test, and cannot be closed proprely.

In some case, it can even provoque failure on following test, maybe
because some cleanup are not done proprely.

This is also breaking the autoretry mecanism since the cursor cannot be
rollbacked, leading to error not related to the first failure.

Part-of: #162990
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Auto retry is mainly useful to avoid the need to rebuild in case of
random failure, but once one test failed, this mecanism is useless,
creates noises in logs, increase laod by retrying test for no reason,
...

This commit should disable this mecanism after the first error.

closes #162990

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Commiting or rollbacking the cursor during test will lead to failure
in cascade, because the cursor cannot be rollbacked at the end of the
test, and cannot be closed proprely.

In some case, it can even provoque failure on following test, maybe
because some cleanup are not done proprely.

This is also breaking the autoretry mecanism since the cursor cannot be
rollbacked, leading to error not related to the first failure.

Part-of: #162990
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Auto retry is mainly useful to avoid the need to rebuild in case of
random failure, but once one test failed, this mecanism is useless,
creates noises in logs, increase laod by retrying test for no reason,
...

This commit should disable this mecanism after the first error.

closes #162990

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Commiting or rollbacking the cursor during test will lead to failure
in cascade, because the cursor cannot be rollbacked at the end of the
test, and cannot be closed proprely.

In some case, it can even provoque failure on following test, maybe
because some cleanup are not done proprely.

This is also breaking the autoretry mecanism since the cursor cannot be
rollbacked, leading to error not related to the first failure.

Part-of: #162990
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Auto retry is mainly useful to avoid the need to rebuild in case of
random failure, but once one test failed, this mecanism is useless,
creates noises in logs, increase laod by retrying test for no reason,
...

This commit should disable this mecanism after the first error.

closes #162990

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Commiting or rollbacking the cursor during test will lead to failure
in cascade, because the cursor cannot be rollbacked at the end of the
test, and cannot be closed proprely.

In some case, it can even provoque failure on following test, maybe
because some cleanup are not done proprely.

This is also breaking the autoretry mecanism since the cursor cannot be
rollbacked, leading to error not related to the first failure.

Part-of: #162990
robodoo pushed a commit that referenced this pull request Apr 29, 2024
Auto retry is mainly useful to avoid the need to rebuild in case of
random failure, but once one test failed, this mecanism is useless,
creates noises in logs, increase laod by retrying test for no reason,
...

This commit should disable this mecanism after the first error.

closes #162990

Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Apr 29, 2024

@Xavier-Do staging failed: ci/l10n on f5ee98ba946ca334c0fcbe457bf24bf6760c7e2e (view more at https://runbot.odoo.com/runbot/build/61986871)

Auto retry is mainly useful to avoid the need to rebuild in case of
random failure, but once one test failed, this mecanism is useless,
creates noises in logs, increase laod by retrying test for no reason,
...

This commit should disable this mecanism after the first error.
Commiting or rollbacking the cursor during test will lead to failure
in cascade, because the cursor cannot be rollbacked at the end of the
test, and cannot be closed proprely.

In some case, it can even provoque failure on following test, maybe
because some cleanup are not done proprely.

This is also breaking the autoretry mecanism since the cursor cannot be
rollbacked, leading to error not related to the first failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants