-
Notifications
You must be signed in to change notification settings - Fork 31
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
Alignment Updates and Exemption Review Unit Tests #1586
Alignment Updates and Exemption Review Unit Tests #1586
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1586 +/- ##
==========================================
+ Coverage 72.35% 77.27% +4.92%
==========================================
Files 307 379 +72
Lines 12767 6976 -5791
Branches 2630 1108 -1522
==========================================
- Hits 9237 5391 -3846
+ Misses 3518 1553 -1965
- Partials 12 32 +20
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/gcbrun |
Temporary Url for review: https://bcregistry-assets-dev--pr-1586-8vgcs4br.web.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
expect(wrapper.findComponent(FolioOrReferenceNumber).exists()).toBe(true) | ||
}) | ||
|
||
it('renders the ConfirmCompletion section', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for having them in separate tests? So we can add more assertions as we go on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, it's just long-hand i guess. Keep it simple.
|
||
export function setupMockQualifiedSupplier (): void { | ||
setupMockUser() | ||
store.setAuthRoles([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably need to specify that this is Lawyer QS.
In my QS tests, I did add [AuthRoles.MHR_TRANSFER_SALE]
roles, with a [ProductCode.MANUFACTURER]
product codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, can update.
Although the Roles no longer have a part to play, for the UI at least. We should just be relying on the product code.
/gcbrun |
Temporary Url for review: https://bcregistry-assets-dev--pr-1586-8vgcs4br.web.app |
Issue #: /bcgov/entity#17420
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the PPR license (Apache 2.0).