[Bug 40046] New: Remove wait and screenshot from Tools/ManageMarcImport_spec.ts
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Bug ID: 40046 Summary: Remove wait and screenshot from Tools/ManageMarcImport_spec.ts Change sponsored?: --- Product: Koha Version: 24.11 Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: jonathan.druart@gmail.com Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |39606 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39606 [Bug 39606] Cover change from bug 39294 with a Cypress test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 182903 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182903&action=edit Bug 40046: (bug 39606 follow-up) Remove waits and screenshots
From bug 39606: """ Why are you taking screenshots? The wait statements will certainly cause random failures.
cy.wait(5000); cy.wait(2000); Notice the following couple of lines: cy.get("#fileuploadstatus").contains("100%"); cy.get("legend").contains("Look for existing records in catalog?").should('be.visible'); It makes Cypress wait 10s until "100%" appears (ie. the file is updated), then the block with the dropdown lists you are going to interact with to be visible. You should almost never use cy.wait. """ Test plan: yarn cypress run --spec t/cypress/integration/Tools/ManageMarcImport_spec.ts should still pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40051 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40051 [Bug 40051] cy.wait(delay) should not be used in Cypress tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182903|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 182912 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182912&action=edit Bug 40046: (bug 39606 follow-up) Remove waits and screenshots
From bug 39606: """ Why are you taking screenshots? The wait statements will certainly cause random failures.
cy.wait(5000); cy.wait(2000); Notice the following couple of lines: cy.get("#fileuploadstatus").contains("100%"); cy.get("legend").contains("Look for existing records in catalog?").should('be.visible'); It makes Cypress wait 10s until "100%" appears (ie. the file is updated), then the block with the dropdown lists you are going to interact with to be visible. You should almost never use cy.wait. """ Test plan: yarn cypress run --spec t/cypress/integration/Tools/ManageMarcImport_spec.ts should still pass Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |pedro.amorim@openfifth.co.u | |k --- Comment #3 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- This is causing tests to fail for me. Consistently. Using UI: assertexpected <select#matcher> to have value '3', but the value was '' AssertionError Timed out retrying after 10000ms: expected '<select#matcher>' to have value '3', but the value was '' Using yarn cypress run --spec t/cypress/integration/Tools/ManageMarcImport_spec.ts: 1) loads the manage MARC import page upload a MARC record: AssertionError: Timed out retrying after 10000ms: Expected to find content: 'View batch' but never did. at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:76:11) Both UI and CLI pass if I revert the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |pedro.amorim@openfifth.co.u |y.org |k -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Pedro Amorim from comment #3)
This is causing tests to fail for me. Consistently. Using UI:
assertexpected <select#matcher> to have value '3', but the value was '' AssertionError Timed out retrying after 10000ms: expected '<select#matcher>' to have value '3', but the value was ''
Using yarn cypress run --spec t/cypress/integration/Tools/ManageMarcImport_spec.ts:
1) loads the manage MARC import page upload a MARC record: AssertionError: Timed out retrying after 10000ms: Expected to find content: 'View batch' but never did. at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:76:11)
Both UI and CLI pass if I revert the patch.
I think it is not related to this patch. It's not consistent for me. After a bit of debug I see that we hit C4/ImportBatch.pm 374 if ( scalar( $marc_record->fields() ) == 0 ) { 375 push @invalid_records, $marc_record; 376 } else { $marc_record is _fields [], _leader " a ", _warnings [ [0] "Record length \"<?xml\" is not numeric in record 1" ] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 182921 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182921&action=edit Bug 40046: Prevent random failures when verifying select's values Commands like .select() are queued. We need to prevent the should to happen before the select is executed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #5)
Created attachment 182921 [details] [review] Bug 40046: Prevent random failures when verifying select's values
Commands like .select() are queued. We need to prevent the should to happen before the select is executed.
This patch fixes another failure: Timed out retrying after 10000ms + expected - actual +'3' at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:67:27) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182921|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 182932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182932&action=edit Bug 40046: Prevent random failures when verifying select's values Commands like .select() are queued. We need to prevent the should to happen before the select is executed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Jonathan Druart from comment #4)
(In reply to Pedro Amorim from comment #3)
This is causing tests to fail for me. Consistently. Using UI:
assertexpected <select#matcher> to have value '3', but the value was '' AssertionError Timed out retrying after 10000ms: expected '<select#matcher>' to have value '3', but the value was ''
Using yarn cypress run --spec t/cypress/integration/Tools/ManageMarcImport_spec.ts:
1) loads the manage MARC import page upload a MARC record: AssertionError: Timed out retrying after 10000ms: Expected to find content: 'View batch' but never did. at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:76:11)
Both UI and CLI pass if I revert the patch.
I think it is not related to this patch.
It's not consistent for me. After a bit of debug I see that we hit C4/ImportBatch.pm 374 if ( scalar( $marc_record->fields() ) == 0 ) {
375 push @invalid_records, $marc_record; 376 } else {
$marc_record is _fields [],
_leader " a ",
_warnings [
[0] "Record length \"<?xml\" is not numeric in record 1"
]
This actually happened when the form was submitted with format=MARC, before MARCXML was actually selected. I had both CLI and UI failing randomly. With this last patch applied they pass consistently. Can you confirm that, Pedro? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|24.11 |unspecified -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #9 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Created attachment 183195 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183195&action=edit UI failure Still fails for me, sorry (same as before). Attached is screenshot of UI failure. CLI failure is the same as before. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Pedro Amorim <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #10 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 183196 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183196&action=edit Bug 40046: Use xml and auto-select it Absolutely no idea why the previous code was not selecting MARCXML (MARC was passed and so the import failed) cy.get('select[name="format"]') .select("MARCXML", { force: true }) .should("have.value", "MARCXML"); But I found a bug in the JS code: we are supposed to auto-detect .xml file and pre-select MARCXML. So let's rely on that behaviour! sample.mrc is actually an xml file so it makes sense to rename it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
From the screenshots and videos it appears the background job fails to start for some reason, possibly low resources on my machine at the time of running
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #11 from Pedro Amorim <pedro.amorim@openfifth.co.uk> --- Been trying this out and chatting with Joubu on Mattermost. It still fails for me often (not always, like 50%) with the same error 'View batch' as before. the tests (?). The background job bar shows '0% failed' in red. Dumping $marc_record in the code mentioned by comment 4 produces the following output, when the tests fail: bless( { '_leader' => ' a ', '_fields' => [], '_warnings' => [ 'Record length "<?xml" is not numeric in record 1' ] }, 'MARC::Record' ); I don't feel comfortable PQAing this for the above reasons but I'm not against it if we decide to push and address this later. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #12 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Pedro Amorim from comment #11)
Been trying this out and chatting with Joubu on Mattermost. It still fails for me often (not always, like 50%) with the same error 'View batch' as before. From the screenshots and videos it appears the background job fails to start for some reason, possibly low resources on my machine at the time of running the tests (?). The background job bar shows '0% failed' in red.
Dumping $marc_record in the code mentioned by comment 4 produces the following output, when the tests fail:
bless( { '_leader' => ' a ', '_fields' => [], '_warnings' => [ 'Record length "<?xml" is not numeric in record 1' ] }, 'MARC::Record' );
I don't feel comfortable PQAing this for the above reasons but I'm not against it if we decide to push and address this later.
What's happening is the form being submitted with "MARC" instead of "MARCXML", even with the last change, which does not make sense at all. So we may have some weird JS code something that reverts to MARC, but I haven't found any. I am suggesting to push as it, to move forward and also unlock the follow-up bug, then add a test at the QA script level to prohibit those cy.wait(\d) that are totally wrong and must (almost) never be used. At least to prevent new occurrences. Then we may want to reintroduce this one back (or simply remove/adjust the tests) if it fails on jenkins. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|pedro.amorim@openfifth.co.u | |k | --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- We need another QA feedback please. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182912|0 |1 is obsolete| | --- Comment #14 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 183827 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183827&action=edit Bug 40046: (bug 39606 follow-up) Remove waits and screenshots
From bug 39606: """ Why are you taking screenshots? The wait statements will certainly cause random failures.
cy.wait(5000); cy.wait(2000); Notice the following couple of lines: cy.get("#fileuploadstatus").contains("100%"); cy.get("legend").contains("Look for existing records in catalog?").should('be.visible'); It makes Cypress wait 10s until "100%" appears (ie. the file is updated), then the block with the dropdown lists you are going to interact with to be visible. You should almost never use cy.wait. """ Test plan: yarn cypress run --spec t/cypress/integration/Tools/ManageMarcImport_spec.ts should still pass Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182932|0 |1 is obsolete| | --- Comment #15 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 183828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183828&action=edit Bug 40046: Prevent random failures when verifying select's values Commands like .select() are queued. We need to prevent the should to happen before the select is executed. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183196|0 |1 is obsolete| | --- Comment #16 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 183829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183829&action=edit Bug 40046: Use xml and auto-select it Absolutely no idea why the previous code was not selecting MARCXML (MARC was passed and so the import failed) cy.get('select[name="format"]') .select("MARCXML", { force: true }) .should("have.value", "MARCXML"); But I found a bug in the JS code: we are supposed to auto-detect .xml file and pre-select MARCXML. So let's rely on that behaviour! sample.mrc is actually an xml file so it makes sense to rename it Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.derscheid@lmscloud.de --- Comment #17 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Ran the tests 5 times, passed 4 out of 5. Hope the first failure was just a hiccup. But as stated in comments, let's see how Jenkins feels about it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Passed QA |Pushed to main Version(s)| |25.11.00 released in| | --- Comment #18 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.11.00 |25.11.00,25.05.02 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #19 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #20 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Had Tools/ManageMarcImport_spec.ts fail the first time again when running the cypress tests locally yesterday. But then passed two subsequent times. Very odd. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 39606 not in 24.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |david@davidnind.com Status|Needs documenting |RESOLVED --- Comment #22 from David Nind <david@davidnind.com> --- Test suite, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #23 from Jonathan Druart <jonathan.druart@gmail.com> --- Koha_Main/3345 02:08:03 koha-1 | <testcase name="loads the manage MARC import page upload a MARC record" time="0.000" classname="upload a MARC record"> 02:08:03 koha-1 | <failure message="Timed out retrying after 10000ms: Expected to find content: 'View batch' but never did." type="AssertionError"><![CDATA[AssertionError: Timed out retrying after 10000ms: Expected to find content: 'View batch' but never did. 02:08:03 koha-1 | at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:75:11)]]></failure> 02:08:03 koha-1 | </testcase> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 --- Comment #24 from Jonathan Druart <jonathan.druart@gmail.com> --- Same failure today Koha_Main/3365 on Docker_14 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.11.00,25.05.02 |25.11.00,25.05.02,24.11.10 released in| | --- Comment #25 from Fridolin Somers <fridolin.somers@biblibre.com> --- Finally backported dependency so Pushed to 24.11.x for 24.11.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41368 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41368 [Bug 41368] Tools/ManageMarcImport_spec.ts is failing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40046 Bug 40046 depends on bug 39606, which changed state. Bug 39606 Summary: Cover change from bug 39294 with a Cypress test https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39606 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org