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

[RFR] Limit analysis verifyStatus timeout to 10 mins #1289

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

Conversation

@mguetta1 mguetta1 changed the title Limit analysis verifyStatus timeout to 10 mins [RFR] Limit analysis verifyStatus timeout to 10 mins Dec 10, 2024
@@ -392,7 +393,7 @@ export class Analysis extends Application {

public static verifyStatus(element: Cypress.Chainable, status: string) {
element
.find("div > div:nth-child(2)", { timeout: 3600000, log: false }) // 1h
.find("div > div:nth-child(2)", { timeout: 10 * MIN, log: false })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some large apps, such as those in ´load_analysis.test.ts´, can take more than 10 minutes to analyze.

You could increase the timeout to 30 minutes or add a default parameter to the verifyStatus method, which can be overridden as needed and set, at least, 30 mins for the apps under ´load_analysis.test.ts´.

Copy link
Contributor Author

@mguetta1 mguetta1 Dec 11, 2024

Choose a reason for hiding this comment

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

Thanks @abrugaro. 30 mins is way too much for an analysis in my opinion. In API tests (go-konveyor-tests repo) the max analysis duration is about 5-6 mins.
I will set the default to 10 mins and for any failure related to this change we will have to investigate if it is indeed a big app or a performance issue

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good!

In that case, could you please trigger a jenkins job to ensure the load_analysis tests does not break after this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, will do it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
@abrugaro I added the Jenkins run to the description

@mguetta1 mguetta1 requested a review from abrugaro December 11, 2024 10:47
@mguetta1 mguetta1 force-pushed the analysis-timeout branch 4 times, most recently from 877e412 to e327c92 Compare December 12, 2024 11:23
Signed-off-by: Maayan Hadasi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants