17765 complete resolution fix #565
Conversation
|
/gcbrun |
|
Temporary Url for review: https://business-create-dev--pr-565-0n72txxt.web.app |
… + moved to jsdom
| @@ -574,7 +574,6 @@ export default class CompleteResolution extends Mixins(CommonMixin, DateMixin) { | |||
| // Note: the image file path did not resolve correctly when using the require function directly. In order | |||
| // to get the image path resolving correctly, needed to get the image context first and use that to build | |||
| // the final image file path | |||
| if (this.isVitestRunning) return '' | |||
There was a problem hiding this comment.
Travis expressed his concerns with this flag. I removed it from here and elsewhere. Unit test was failing:
After doing some research (mswjs/msw#1521), I was able to make the unit tests run by moving from happy-dom to jsdom.
@severinbeauvais We might want to do the same across all the UIs to stay consistent.
Note: The isVitestRunning method is still being used in one place (App.vue). For whatever reason, without it, one test fails which is expect(wrapper.findComponent(SbcFeeSummary).exists()).toBe(true). https://github.com/JazzarKarim/business-create-ui/blob/8fab017397867b6e99f862b4e848af095aac1bc3/tests/unit/App.spec.ts#L1094
There was a problem hiding this comment.
How about changing to jsdom as needed? In theory, happy-dom is faster or better or something (happier?).
There was a problem hiding this comment.
Oh, you mean leave jsdom here and keep happy-dom in the other UIs? Sure! That works too 🙌
There was a problem hiding this comment.
Yes, leave the others until we have a reason to update them.
There was a problem hiding this comment.
All right, sounds good.
| // eslint-disable-next-line accessor-pairs | ||
| Object.defineProperty(window.location, 'href', { | ||
| set (url: string) { | ||
| redirectedUrl = url |
There was a problem hiding this comment.
This test was failing after moving from happy-dom to jsdom:
The new code fixes it. Reference: jestjs/jest#5124
|
/gcbrun |
|
Temporary Url for review: https://business-create-dev--pr-565-0n72txxt.web.app |
| @@ -286,8 +285,8 @@ export default class Actions extends Mixins(CommonMixin, DateMixin, FilingTempla | |||
| this.setIsFilingPaying(false) | |||
| } | |||
| } else { | |||
| // don't call window.scrollTo during Vitest tests because happy-dom doesn't implement it | |||
| if (!this.isVitestRunning) window.scrollTo({ top: 0, behavior: 'smooth' }) | |||
| // don't call window.scrollTo during Vitest tests because jsdom doesn't implement it | |||
There was a problem hiding this comment.
Hehe, obsolete comment.
Issue #: /bcgov/entity#17765
Description of changes:
CompleteResolution.vueafter the latest hotfix has been deployedBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).