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

[FIX] test: non deterministic test #67374

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

william-andre
Copy link
Contributor

When creating a test, for instance (the bug was in master but it makes sense to fix it in 14.0 IMO)

@freeze_time("2020-11-30 19:45:00")
class TestNacha(AccountTestInvoicingCommon):
    @classmethod
    def setUpClass(cls, chart_template_ref='l10n_generic_coa.configurable_chart_template'):
        super().setUpClass(chart_template_ref=chart_template_ref)

        cls.company_data["default_journal_bank"].write({
            "nacha_immediate_destination": "IMM_DESTINATION",
            "nacha_immediate_origin": "IMM_ORIGIN",
            "nacha_destination": "DESTINATION",
            "nacha_company_identification": "COMPANY_ID",
            "nacha_origination_dfi_identification": "ORIGINATION_DFI",
        })

        cls.bank = cls.env["res.partner.bank"].create({
            "partner_id": cls.partner_a.id,
            "acc_number": "987654321",
            "aba_routing": "123456789",
        })

        def create_payment(partner, amount, ref):
            return cls.env['account.payment'].create({
                "partner_id": partner.id,
                "partner_bank_id": cls.bank.id,
                "ref": ref,
                "amount": amount,
                "payment_type": "outbound",
                "date": datetime.datetime.today(),
            })
        cls.batch = cls.env["account.batch.payment"].create({
            "journal_id": cls.company_data["default_journal_bank"].id,
            "batch_type": "outbound",
            "payment_ids": [
                Command.link(create_payment(cls.partner_a, 123, 'test1').id),
                Command.link(create_payment(cls.partner_b, 456, 'test2').id),
            ]
        })

    def testGenerateNachaFile(self):
        expected = [
            # header
            "101IMM_DESTINIMM_ORIGIN2011301945A094101DESTINATION            company_1_data         {:8d}".format(self.batch.id),
            # batch header for payment "test1"
            "5225company_1_data                      COMPANY_IDPPDtest1     201130201130   1ORIGINAT0000000",
            # entry detail for payment "test1"
            "627123456789987654321        0000012300               partner_a               0ORIGINAT0000000",
            # batch control record for payment "test1"
            "82250000010000000036000000012300000000000000COMPANY_ID                         ORIGINAT0000000",
            # batch header for payment "test2"
            "5225company_1_data                      COMPANY_IDPPDtest2     201130201130   1ORIGINAT0000001",
            # entry detail for payment "test2"
            "627123456789987654321        0000045600               partner_b               0ORIGINAT0000000",
            # batch control record for payment "test2"
            "82250000010000000036000000045600000000000000COMPANY_ID                         ORIGINAT0000001",
            # file control record
            "9000002000004000000020000000072000000000579000000000000                                       ",
        ]

        for generated, expected in zip(self.batch._generate_nacha_file().splitlines(), expected):
            self.assertEqual(generated, expected, "Generated line in NACHA file does not match expected.")

We have a non deterministic error because the Command.link is using a python set, which is not ordered[1]
We are then setting a random order on the field stored in the x2m field when setting it. We won't have that issue in other transactions because we will then have to populate the cache again, which will take care of the _order.

I have identified 4 ways to correct that.

  • The first commit
  • The second commit
  • Calling self.env.cache.invalidate() in the test case
  • Calling .sorted() in the business code (which will be useless in all cases outside of the test)

[1] https://stackoverflow.com/questions/45581901/are-sets-ordered-like-dicts-in-python3-6
https://docs.python.org/3/tutorial/datastructures.html#sets

A set is an unordered collection with no duplicate elements

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo
Copy link
Contributor

robodoo commented Mar 5, 2021

@C3POdoo C3POdoo added the RD research & development, internal work label Mar 5, 2021
@william-andre william-andre changed the base branch from 14.0 to master November 15, 2022 15:23
@C3POdoo C3POdoo requested review from a team November 15, 2022 15:36
Copy link
Member

@Julien00859 Julien00859 left a comment

Choose a reason for hiding this comment

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

I was worrying about clearing the env/cache twice in a row (setUp+cleanUp) but it turns out clear_caches() and clear() are basically no-op when the underlying datastructures are empty already.

The runbot it red though

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

5 participants