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

Feature/294 clear test datasets #360

Merged
merged 7 commits into from
Apr 16, 2024
Merged

Conversation

ekraffmiller
Copy link
Contributor

What this PR does / why we need it:

Deletes test datasets before e2e tests, so that destroyAll() doesn't fail when trying to delete too many tests at once.

Which issue(s) this PR closes:

Special notes for your reviewer:

Suggestions on how to test this:

Run the e2e tests, check that test datasets have been deleted after the tests run. Check that "gets the total dataset count" test in DatasetJSDataverseRepository.spec.ts runs without errors

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

no

Is there a release notes update needed for this change?:

Additional documentation:

@ekraffmiller ekraffmiller marked this pull request as ready for review April 1, 2024 15:00
@ekraffmiller ekraffmiller added pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 3 A percentage of a sprint. 2.1 hours. labels Apr 1, 2024
@MellyGray MellyGray self-assigned this Apr 2, 2024
Copy link
Contributor

@MellyGray MellyGray 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 also creating datasets in the FileJSDataverseRepository.spec.ts and Collection.spec.tsx files. We should include the destroyAll method there as well.

Additionally, since we're now destroying all datasets at the beginning of each test, we should consider removing all calls to the destroyAll method within the individual tests. Such as in cy.wrap(DatasetHelper.destroyAll().then(() => DatasetHelper.createWithTitle(title)))

@MellyGray MellyGray assigned ekraffmiller and unassigned MellyGray Apr 2, 2024
@coveralls
Copy link

coveralls commented Apr 2, 2024

Coverage Status

coverage: 97.694%. remained the same
when pulling d32bb54 on feature/294-clear-test-datasets
into 3aa4685 on develop.

@ekraffmiller
Copy link
Contributor Author

thanks @MellyGray, I made the changes you suggested

MellyGray
MellyGray previously approved these changes Apr 3, 2024
Copy link
Contributor

@MellyGray MellyGray left a comment

Choose a reason for hiding this comment

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

LGTM!

@g-saracca g-saracca self-assigned this Apr 3, 2024
@g-saracca
Copy link
Contributor

Hi @ekraffmiller , 2 suites of tests are failing, I attach screenshots.
I removed images and volumes from docker, downloaded everything again and they are still occurring in the same way.
Screenshot 2024-04-03 at 09 22 03
Screenshot 2024-04-03 at 08 54 04
Screenshot 2024-04-03 at 09 21 01

@g-saracca g-saracca assigned ekraffmiller and unassigned g-saracca Apr 3, 2024
@ekraffmiller
Copy link
Contributor Author

@GermanSaracca I tried it again locally, as well as re-running the action, and I haven't reproduced the errors. I think it must be a timing issue. One thing that may be different is the Docker settings. A while ago, I had to increase my resource settings in Docker, in order to run other containers. Here is what I have for Docker resources:
Screenshot 2024-04-03 at 11 57 35 AM
Can you check what you have, and maybe try running again with more memory?

@g-saracca g-saracca removed their assignment Apr 4, 2024
@g-saracca
Copy link
Contributor

I unassign me from possible local environment problems to test this issue. Looks like a timing issue on my end.
@MellyGray @GPortas could any of you test this on your end?

@GPortas GPortas self-assigned this Apr 12, 2024
@g-saracca g-saracca assigned g-saracca and unassigned g-saracca Apr 12, 2024
Copy link
Contributor

@GPortas GPortas left a comment

Choose a reason for hiding this comment

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

@ekraffmiller Can you please resolve the merge conflicts?

@GPortas GPortas merged commit 6849096 into develop Apr 16, 2024
14 checks passed
@GPortas GPortas deleted the feature/294-clear-test-datasets branch April 16, 2024 10:47
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clear test datasets before integration test
5 participants