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 cypress tests for files_versions #37345

Merged
merged 12 commits into from
Apr 4, 2023

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Mar 22, 2023

In files_versions cypress tests, we wait for 1000ms between each versions uploads to make sure that the server properly create a version. Under 1s the serve does not create a version.

This commit increases the wait time as it might help to ensure that the time span is enough between two uploads.

The automatic versions expiration process was kicking in during the tests, making some versions unavailable. I changed the tests to isolate expiration process testing from other tests.

@artonge artonge self-assigned this Mar 22, 2023
@artonge artonge added bug 3. to review Waiting for reviews tests Related to tests labels Mar 22, 2023
@artonge artonge changed the title Artonge/fix_files_version_cypress_tests Increase wait to ensure versions are created Mar 22, 2023
@artonge artonge changed the title Increase wait to ensure versions are created Increase wait time to ensure versions are created Mar 22, 2023
@skjnldsv
Copy link
Member

Ok, but with one comment :)

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

We're not using docker compose?

@artonge artonge force-pushed the artonge/fix_files_version_cypress_tests branch 4 times, most recently from a7b24de to f6084c9 Compare March 28, 2023 11:34
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 28, 2023
@artonge artonge force-pushed the artonge/fix_files_version_cypress_tests branch from 4207857 to 13f76e0 Compare March 29, 2023 15:07
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

artonge added 10 commits March 30, 2023 10:09
In files_versions cypress tests, we wait for 1000ms between each versions uploads to make sure that the server properly create a version. Under 1s the serve does not create a version.

This commit increases the wait time as it might help to ensure that the time span is enough between two uploads.

Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
@artonge artonge force-pushed the artonge/fix_files_version_cypress_tests branch from 8de755f to ae0ec6f Compare March 30, 2023 08:09
@artonge
Copy link
Contributor Author

artonge commented Mar 30, 2023

Strongly suspecting auto expiration from deleting files, let's see

@artonge artonge force-pushed the artonge/fix_files_version_cypress_tests branch from 128955a to f0a1163 Compare March 30, 2023 14:59
@artonge artonge force-pushed the artonge/fix_files_version_cypress_tests branch from f0a1163 to 5dc64eb Compare March 30, 2023 15:03
Copy link
Contributor Author

@artonge artonge left a comment

Choose a reason for hiding this comment

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

@skjnldsv I managed to get the tests working again. I was tricked by the auto expiration process...

The culprit is that I move the occ config:system:set versions_retention_obligation call from dockerNode.ts to version_expiration.cy.ts.

Note that:

  • I extended the cypress workflow to extract logs and data directory. Can be remove but seems useful. Ok to keep?
  • The change also contains logic to use a random file name instead of 'test.txt'. Not needed, but it's here and it works.

Comment on lines +74 to +76
if (!process.env.CI) {
stopNextcloud()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to extract logs and data directory

@artonge artonge changed the title Increase wait time to ensure versions are created Fix cypress tests for files_versions Apr 4, 2023
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

It should only upload log and data on failure, no?

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@nickvergessen nickvergessen enabled auto-merge April 4, 2023 09:35
@nickvergessen nickvergessen merged commit 44cb46c into master Apr 4, 2023
@delete-merged-branch delete-merged-branch bot deleted the artonge/fix_files_version_cypress_tests branch April 4, 2023 11:43
@artonge
Copy link
Contributor Author

artonge commented Apr 4, 2023

/backport to stable26

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin/stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug tests Related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants