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(STONEBLD-2265): increase the default timeout of pull-request-type PLR & e2e-tests Task to 2hrs #893

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

dheerajodha
Copy link
Member

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

.tekton/pull-request.yaml Outdated Show resolved Hide resolved
.tekton/pull-request.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

I tried to do something similar in 4243774, but I still saw tests getting interrupted after roughly an hour.

But it looked like a timeout on the ginkgo side, not an openshift timeout. I tried setting the ginkgo.timeout flag but not sure if it did anything

@MartinBasti
Copy link
Contributor

@chmeliik konflux-ci/e2e-tests#1091 could this help?

@MartinBasti MartinBasti reopened this Mar 20, 2024
@chmeliik
Copy link
Contributor

chmeliik commented Mar 21, 2024

@chmeliik redhat-appstudio/e2e-tests#1091 could this help?

Hmm, the timeouts I was seeing were not from waiting for the attestation, they looked like overall timeouts for the whole test suite (but coming from ginkgo, not from openshift). I unfortunately don't have the error message saved anywhere, so let's just go ahead with this and see if we hit timeouts again

@dheerajodha dheerajodha force-pushed the STONEBLD-2265 branch 2 times, most recently from 6ebe771 to c4fd5df Compare March 21, 2024 08:59
Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@dheerajodha dheerajodha changed the title fix(STONEBLD-2265): increase the default timeout of pull-request-type PLR & e2e-tests Task to 90mins fix(STONEBLD-2265): increase the default timeout of pull-request-type PLR & e2e-tests Task to 2hrs Mar 21, 2024
@dheerajodha
Copy link
Member Author

/retest

@chmeliik
Copy link
Contributor

@dheerajodha if this round of tests doesn't pass, can you try to combine this PR with #897? I think there's a low chance of passing tests otherwise

@MartinBasti
Copy link
Contributor

Can we just merge it? it only affects tekton definitions and that's working

@dheerajodha
Copy link
Member Author

@dheerajodha if this round of tests doesn't pass, can you try to combine this PR with #897? I think there's a low chance of passing tests otherwise

Oh nice, done, PTAL. I didn't know that I had to update the image but it makes sense now, thanks for letting me know!

Can we just merge it? it only affects tekton definitions and that's working

Can we just wait for one last build PLR to finish, please?
I just want to see the changes taking effect once :peepo-shy:

@chmeliik
Copy link
Contributor

Hmm, yeah here's the suite timeout

[e2e-tests : e2e-test] Summarizing 1 Failure:
[e2e-tests : e2e-test]   [TIMEDOUT] [build-service-suite Build templates E2E test] HACBS pipelines when the container image for component with Git source URL https://github.com/redhat-appstudio-qe/devfile-sample-python-basic is created and pushed to container registry [It] verify-enterprise-contract check should pass [build, build-templates, HACBS, pipeline, sbom, slow, build-templates-e2e]
[e2e-tests : e2e-test]   /github.com/redhat-appstudio/e2e-tests/tests/build/build_templates.go:385
[e2e-tests : e2e-test] 
[e2e-tests : e2e-test] Ran 17 of 759 Specs in 3601.287 seconds
[e2e-tests : e2e-test] FAIL! - Suite Timeout Elapsed -- 16 Passed | 1 Failed | 10 Pending | 732 Skipped
[e2e-tests : e2e-test] --- FAIL: TestE2E (3601.44s)
[e2e-tests : e2e-test] FAIL

@chmeliik
Copy link
Contributor

@dheerajodha oh, I think I found the timeout flag

$ ginkgo help ginkgo | grep timeout
  --timeout [duration] (default: 1h)
    Test suite fails if it does not complete within the specified timeout.

Needs to be added to .tekton/tasks/e2e-test.yaml

@chmeliik
Copy link
Contributor

@dheerajodha oh, I think I found the timeout flag

$ ginkgo help ginkgo | grep timeout
  --timeout [duration] (default: 1h)
    Test suite fails if it does not complete within the specified timeout.

Needs to be added to .tekton/tasks/e2e-test.yaml

Hmm, no, it should be --ginkgo.timeout like I had in 4243774... but that didn't seem to work 😕

$ podman run --rm -ti quay.io/redhat-appstudio/e2e-tests:c18b86ccadcc8818d8d0e6e07d2dd4e98c7b5af8 --help | grep timeout
  --ginkgo.timeout [duration] (default: 1h)
    Test suite fails if it does not complete within the specified timeout.
  -test.timeout d
    	panic test binary after duration d (default 0, timeout disabled)

Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

Anyway, we can merge and play with the --ginkgo.timeout in a follow-up

dheerajodha and others added 2 commits March 21, 2024 16:06
* This change increases the default timeout (60mins) of the
  PR-type PLR and the 'e2e-tests' Task to 2hrs.
* This is a temporary change, please see:
  konflux-ci/e2e-tests#1091
* Since the chains controller is overwhelmed, we had to
  increase the test timeout value to 90mins.
* So this current PR increases the timeout values for PLR
  and a Task to the 2hrs to give enough time for chains
  as well as other tests to execute within time.

Signed-off-by: Dheeraj<[email protected]>
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@mmorhun mmorhun merged commit d07462a into konflux-ci:main Mar 21, 2024
5 of 6 checks passed
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.

5 participants