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.

Sign up for GitHub

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

Jump to bottom

Improve test documentation (VII): Shared tests (Aliases) #2441

Merged
merged 4 commits into from Jun 13, 2022
Merged

Improve test documentation (VII): Shared tests (Aliases) #2441

merged 4 commits into from Jun 13, 2022

Conversation

reosarevok
Copy link
Member

@reosarevok reosarevok commented Mar 4, 2022 •
edited

I'm changing a lot of tests that we already had for several entities, because it seems a lot easier to see all the updates to them in one commit each rather than slowly doing them per entity.

See commit messages for further details.

@reosarevok reosarevok added Documentation PRs that mostly create or update documentation Tests PRs that mostly create or update tests labels Mar 4, 2022
@reosarevok reosarevok force-pushed the document-tests-7 branch 2 times, most recently from 0d8e06a to d3f7a62 Compare March 4, 2022 16:33
@reosarevok reosarevok changed the title Improve test documentation (VII): Controller::Label (and equivalent tests for other entities) Improve test documentation (VII): Controller::Label start (and equivalent tests for other entities) Mar 7, 2022
@reosarevok reosarevok marked this pull request as ready for review March 7, 2022 09:00
@reosarevok reosarevok force-pushed the document-tests-7 branch 2 times, most recently from 2497bc1 to 3d7f34a Compare March 11, 2022 08:37
@reosarevok reosarevok changed the title Improve test documentation (VII): Controller::Label start (and equivalent tests for other entities) Improve test documentation (VII): Shared tests (Aliases) Mar 11, 2022
mwiencek
mwiencek approved these changes Jun 10, 2022
View reviewed changes
Copy link
Member

@mwiencek mwiencek left a comment

Choose a reason for hiding this comment

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

Overall this looks like a massive improvement, though I would write some of the descriptions slightly differently

t/lib/t/MusicBrainz/Server/Controller/Label/AddAlias.pm Outdated Show resolved Hide resolved
t/lib/t/MusicBrainz/Server/Controller/Label/AddAlias.pm Outdated Show resolved Hide resolved
@reosarevok
Document and standardize Controller::AddAlias tests
c2e1509
@reosarevok
Document and standardize Controller::DeleteAlias tests
72460c0 
Changed the series alias because otherwise
the series name check clashed with the alias name one.
@reosarevok
Document and standardize Controller::EditAlias tests
2bac2b8 
Changed the recording and series alias types for consistency, since
they were set to search hint which has its own constraints.
Added an extra test to intentionally check that changing an alias
to be a search hint does get those constraints applied,
since that also seems useful to test.
@reosarevok
Document and standardize Controller::Aliases tests
64c12ba 
Added alias types where needed so that the tests will be more complete.
For RG, marking one as search hint also allows us to check
that search hints do *not* appear in JSON-LD, as intended.
For some reason, we do not do JSON-LD for series.
@reosarevok reosarevok merged commit 2980adc into metabrainz:master Jun 13, 2022
@reosarevok reosarevok deleted the document-tests-7 branch June 13, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwiencek mwiencek mwiencek approved these changes

Assignees
No one assigned
Labels
Documentation PRs that mostly create or update documentation Tests PRs that mostly create or update tests
Projects
None yet
Milestone
No milestone
2 participants
@reosarevok @mwiencek