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: add better policy step tests #1224

Merged
merged 4 commits into from
Oct 13, 2023
Merged

Conversation

jaredgalanis
Copy link
Contributor

No description provided.

- the factories are filling in data where the fixtures are silent, so the pmcParticipation field on journal was being filled in as A by default.
- we should be explicit when creating the journal via the type in method in the journal field in the acceptance tests to pass in pmcPartificpation method
@jaredgalanis jaredgalanis requested a review from jabrah October 13, 2023 11:02
assert.ok(journal);
});

test('it computes pmcParticipation correctly', function (assert) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this might be the only test that was missing entirely that addresses that regresssion

@@ -285,7 +285,10 @@ export default function (config) {
issns: ['Print:0003-2654', 'Online:1364-5528'],
journalName: 'The Analyst',
nlmta: 'Analyst',
pmcParticipation: 'B',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to add this explicitly because otherwise, this default https://github.com/eclipse-pass/pass-ui/pull/1224/files#diff-cd49b43c6fe4ed8bd896068427733aa3dab8fb10d917ab6bf67a0c325b0a6811R7 in the factory overrrides it.


const submissionAttrs = this.server.create('submission').attrs;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a pretty common mirage pattern where you aren't actually making a network request to get data and instead are just mocking data for a component. That is to say: create the record in the test db with the factory then grab the attrs so you can use those in creating a record in the store.

description: 'This is a moo-scription',
title: 'Moo title',
id: 1,
const submissionAttrs = this.server.create('submission').attrs;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty much all of the changes in this file involve moving to mirage and factories for the mock data and then also using the new data test selectors to be more declarative about what is being tested.

@jaredgalanis jaredgalanis merged commit 86f3fc3 into main Oct 13, 2023
2 checks passed
@jaredgalanis jaredgalanis deleted the add-regression-test-pmc branch October 13, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants