Summary: | TestBuilder fragile on virtual fks | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi <tomascohen> |
Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | dcook, jonathan.druart, kyle, lucas, m.de.rooy, martin.renvoize, nick, nugged |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16155 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.11.00
|
|
Bug Depends on: | 24857, 30275 | ||
Bug Blocks: | |||
Attachments: |
Bug 31133: Regression tests (1)
Bug 31133: Add a way to handle multiple relationships on same column [ALTERNATIVE-PATCH] Bug 31133: Don't generate values for more than one FK [ALTERNATIVE-PATCH] Bug 31133: Don't generate values for more than one FK Bug 31133: Don't generate values for more than one FK |
Description
Tomás Cohen Arazi
2022-07-11 23:29:33 UTC
Created attachment 137599 [details] [review] Bug 31133: Regression tests (1) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 137600 [details] [review] Bug 31133: Add a way to handle multiple relationships on same column This patch introduces a syntax for TestBuilder so we can specify which is the preferred relationship in situations like Koha::Checkouts::Renewals that have more than one virtual FK [1] for the same column. In this case, there are the following relationships: * checkouts * old_checkouts both of which link the *checkout_id* column to the *issue_id* column of the **issues** and **old_issues** tables respectively. It does so by defining a mapping between the (concatenated, maybe multiple) related source column and foreign counterpart. The mapping structure is explained in the POD. To test: 1. Apply the regression tests 2. Run the tests a couple times: $ kshell k$ prove t/db_dependent/TestBuilder_multi_vfk.t => FAIL: Tests fail often, randomly! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! No random failures! 5. Bonus, run: $ kshell k$ prove t/db_dependent/TestBuilder.t => FAIL: It fails consistently about Koha::Biblio::ItemGroup [1] i.e. defined at DBIC level but not present on the DB structure. DISCLAIMER: This is not a complete solution as it still requires a way to override the hardcoded default. This would require a special syntax for build() and build_object() which I prefer to delay until we really need it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Ok, this should cover one part of the problem. There's still Koha::Biblio::ItemGroup which is failing, ran out of fuel today/yesterday, late already :-D Did you notice that "Bug 30275: (QA follow-up) Trivial fix to schema file" fixes this problem? (In reply to Jonathan Druart from comment #4) > Did you notice that "Bug 30275: (QA follow-up) Trivial fix to schema file" > fixes this problem? Hum no, it did not! Created attachment 137603 [details] [review] [ALTERNATIVE-PATCH] Bug 31133: Don't generate values for more than one FK What about this alternative, Tomas? At least we won't have more code to add for other specific cases. That's pretty elegant and simple.. I like Created attachment 137604 [details] [review] [ALTERNATIVE-PATCH] Bug 31133: Don't generate values for more than one FK It was not working, randomly failing! The second patch should be correct. Note that I don't get any other failures, t/db_dependent/TestBuilder.t is passing. Note that I don't get any other failures, t/db_dependent/TestBuilder.t is passing.(In reply to Jonathan Druart from comment #11) > Note that I don't get any other failures, t/db_dependent/TestBuilder.t is > passing. Wrong! Hard morning... Can't use string ("Koha::Biblio::ItemGroup") as a HASH ref while "strict refs" in use at /kohadevbox/koha/Koha/Object.pm line 830. (In reply to Tomás Cohen Arazi from comment #3) > Ok, this should cover one part of the problem. There's still > Koha::Biblio::ItemGroup which is failing, ran out of fuel today/yesterday, > late already :-D I've attached a patch on bug 24857 for that: "Bug 24857: Remove object_class from singular classes". My first implementation was similar to Jonathan's. The 'problem' was that it randomly generated old_issues and it felt like a very rare use case. It felt like it was better to just have a default resolution path. I'm fine with either. Created attachment 137634 [details] [review] Bug 31133: Don't generate values for more than one FK Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 22.11. Nice work everyone, thanks! |