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

Increase timeout when looking for user in list #31072

Closed
wants to merge 1 commit into from

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Feb 8, 2022

My empirical findings show that https://github.com/nextcloud/server/blob/master/tests/acceptance/features/users.feature#L48 is causing half of the failures of acceptances tests in CI.

My assumption is that the server is not powerful enough and that a 100 seconds timeout is too short. But without more context like screenshots on failures, it is difficult to know for sure. Also, it is hard to reproduce locally as this does not fail all the time.

This PR simply increase the timeout to 150 seconds, hoping that this will fix the regular CI failures and help us having more trust in our CI tests.

Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge self-assigned this Feb 8, 2022
@artonge artonge requested review from a team, nickvergessen, juliusknorr and come-nc and removed request for a team February 8, 2022 11:41
@artonge
Copy link
Contributor Author

artonge commented Feb 8, 2022

Ironically the concerned test is failing in this PR...

@nickvergessen
Copy link
Member

Ironically the concerned test is failing in this PR...

I think the problem is more that the selector sometimes does not match. Maybe it fails to switch to the disabled user tab or something?

@come-nc
Copy link
Contributor

come-nc commented Feb 8, 2022

I know that on some test, not sure if it was this one, I once thought that the problem was the password timeout, because in some cases Nextcloud will open a pop-up to ask for your password to confirm some actions, but only if there have been some time since last time the password was entered. I was wondering if this was not the problem with the test, sometimes it would be too long after login and this pop-up would mess up with the selector. Just a theory though. (I thought about that because when attempting to do the same thing as the test by hand I had the pop-up in the place the test failed)

@nickvergessen
Copy link
Member

Password timeout is 30 minutes

@artonge
Copy link
Contributor Author

artonge commented Feb 8, 2022

I think the problem is more that the selector sometimes does not match. Maybe it fails to switch to the disabled user tab or something?

I thought about that, but from looking at the element selection code, it looks like it would fail beforehand with an exception.

sometimes it would be too long after login and this pop-up would mess up with the selector.

Could be, but as said nickvergessen the timeout is 30 minutes. Do you have the example of fixing a test by handling this popup?

@come-nc
Copy link
Contributor

come-nc commented Feb 8, 2022

Could be, but as said nickvergessen the timeout is 30 minutes. Do you have the example of fixing a test by handling this popup?

No, this was just an idea I had because once when trying to reproduce a failure by hand I had this password pop-up in the same place the test failed. But it may be a wrong idea if the timeout is 30minutes.

@nickvergessen
Copy link
Member

Maybe start checking if other users are there, or check if the popup is shown, etc?

@MichaIng MichaIng added this to the Nextcloud 24 milestone Feb 15, 2022
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
This was referenced Apr 7, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@artonge
Copy link
Contributor Author

artonge commented Jun 13, 2022

FYI, this was an attempt, but it does not work

@artonge artonge marked this pull request as draft June 13, 2022 08:29
@PVince81 PVince81 closed this Jun 13, 2022
@artonge artonge removed this from the Nextcloud 25 milestone Jun 13, 2022
@skjnldsv skjnldsv deleted the fix/acceptance_tests branch March 14, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants