Skip to content

[18.0][IMP] product_cost_security: Make tests more resilient#2254

Open
sergio-teruel wants to merge 1 commit intoOCA:18.0from
Tecnativa:18.0-IMP-product_cost_security-resilient-tests
Open

[18.0][IMP] product_cost_security: Make tests more resilient#2254
sergio-teruel wants to merge 1 commit intoOCA:18.0from
Tecnativa:18.0-IMP-product_cost_security-resilient-tests

Conversation

@sergio-teruel
Copy link
Copy Markdown
Contributor

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hi @yajo, @rafaelbn,
some modules you are maintaining are being modified, check this out!

Copy link
Copy Markdown
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +64 to +71
if (
config["test_enable"]
and not self.env.context.get("testing_product_cost_security")
and "standard_price" in fields
):
# By default in other tests users have not this security group, so do this
# only when we are testing this module.
return fields
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The logic of this module is not exclusive to the standard_price field; it applies to every field that has the group product_cost_security.group_product_cost defined in its field definition. So, I think it is better to use the function _product_cost_security_fields to check this.

Additionally, I returned the super() call.
What do you think about this approach?

Suggested change
if (
config["test_enable"]
and not self.env.context.get("testing_product_cost_security")
and "standard_price" in fields
):
# By default in other tests users have not this security group, so do this
# only when we are testing this module.
return fields
product_cost_fields = self._product_cost_security_fields().intersection(
valid_fields
)
if (
config["test_enable"]
and not self.env.context.get("testing_product_cost_security")
and product_cost_fields
):
# By default in other tests users have not this security group, so do this
# only when we are testing this module.
return valid_fields

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion: alter context in test only once, here:

Suggested change
cls.env = cls.env(context=dict(cls.env.context, disable_auto_svl=True, testing_product_cost_security=True))

@sergio-teruel sergio-teruel force-pushed the 18.0-IMP-product_cost_security-resilient-tests branch from 34936be to f118288 Compare April 18, 2026 05:24
@OCA-git-bot OCA-git-bot added series:18.0 mod:product_cost_security Module product_cost_security labels Apr 18, 2026
@sergio-teruel
Copy link
Copy Markdown
Contributor Author

@yajo @carlos-lopez-tecnativa Changes done!

@sergio-teruel
Copy link
Copy Markdown
Contributor Author

Do not merge, the last changes not fix the issue

@sergio-teruel sergio-teruel force-pushed the 18.0-IMP-product_cost_security-resilient-tests branch from f118288 to c3db4b2 Compare April 22, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants