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

use Valkryie for all test setup in edit_permissions_service_spec #6172

Merged
merged 4 commits into from
Aug 25, 2023

Conversation

no-reply
Copy link
Contributor

makes this test compatible with Valkyrie by refactoring out ActiveFedora based
factories.

the methodology i used here is to:

  • stand up dassie;
  • refactor for tidiness/to remove AF based mocks;
  • rewrite with Valkyrie factories;
  • test on dassie;
  • test on koppie

@samvera/hyrax-code-reviewers

@no-reply no-reply force-pushed the refactor-edit-perm-serv-spec branch from ac0c9be to 6a4d5a4 Compare August 23, 2023 16:59
tamsin johnson added 3 commits August 23, 2023 15:30
this is a model heavy test, and yes, the setup is slow, but that's a code smell
we shouldn't be hiding with convoluted test setup.
makes this test compatible with Valkyrie by refactoring out ActiveFedora based
factories.

the methodology i used here is to:

  - stand up dassie;
  - refactor for tidiness/to remove AF based mocks;
  - rewrite with Valkyrie factories;
  - test on dassie;
  - test on koppie
@no-reply no-reply force-pushed the refactor-edit-perm-serv-spec branch from 6a4d5a4 to d6b370b Compare August 23, 2023 22:30
Copy link
Contributor

@dlpierce dlpierce left a comment

Choose a reason for hiding this comment

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

Great example of converting a spec. Thanks!

@dlpierce dlpierce merged commit 302e037 into main Aug 25, 2023
@dlpierce dlpierce deleted the refactor-edit-perm-serv-spec branch August 25, 2023 18:23
@dlpierce dlpierce added the notes-valkyrie Release Notes: Valkyrie specific label Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-valkyrie Release Notes: Valkyrie specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants