-
Notifications
You must be signed in to change notification settings - Fork 64
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
Moving from avoriaz #173
Labels
Comments
matthew-white
added a commit
that referenced
this issue
Jun 23, 2021
Closes #173. Notes: - avoriaz has some information on migrating to Vue Test Utils (VTU): https://github.com/eddyerburgh/avoriaz/blob/master/docs/guides/migrating-to-vue-test-utils.md - VTU wrappers no longer have a data() method. Using the `vm` property to access data properties instead. - Replace wrapper methods: - avoriaz() getProp() becomes VTU props() - hasClass() becomes classes() - getAttribute() and hasAttribute() become attributes() - first() becomes get() or getComponent() - find() becomes findAll() or findAllComponents() - Unless doing an existence check, in which case we call either find() or findComponent(), then call exists(). - For a wrapper of a Vue component, access the element using the `element` property rather than using vm.$el. - The VTU wrapper method text() trims the text. The avoriaz wrapper method did not, so we often called trim() after text(). We now access element.textContent directly in the rare case that we do not want the text to be trimmed. - The VTU wrapper method html() prettifies the HTML, which the avoriaz wrapper method did not. That may be useful for us in the future, but for our few existing tests that used the method (in SubmissionFeedEntry), we now use element.outerHTML instead. - Replace our `trigger` object with the VTU wrapper methods trigger(), setValue(), and setChecked(). - But keeping a version of trigger.dragAndDrop(), in the new test/util/file.js. - The methods of the `trigger` object returned a promise that resolved to the avoriaz wrapper. The VTU wrapper methods return a promise that doesn't resolve to a value, so I had to restructure some promise chains. In many cases, I rewrote the promise chain using async/await. - It is now possible to specify options when using VTU to trigger an event, so we don't need to use jQuery to mock events. I removed jQuery from FormNew and FormAttachmentUploadFiles, which only used jQuery so that we could mock events in testing. - The tests of FormNew have good examples of some of these changes. - Use the VTU wrapper method emitted() instead of using Sinon to fake $emit(). (The tests of DateRangePicker have good examples of this change.) - Replace attachToDocument with attachTo. - avoriaz allowed a wrapper to be passed as a slot, but VTU does not. However, VTU allows a template string to be passed, which avoriaz did not. The only place this comes up is a test of LinkIfCan, where the wrapper could be replaced with a template string. - Use the parentComponent option of mount() to account for i18n custom blocks. Previously, we overwrote the setProps() wrapper method. - We inject the router in fewer tests. In some cases, we are able to switch from load() to mount(), making the test more isolated and often making it synchronous. (The tests of Download and FormDraftChecklist have good examples of that.) - If a component uses <router-link>, VTU can stub that. - If a component uses $route, VTU can mock that. - In order for VTU to mock $route, we use createLocalVue() to create a copy of Vue with the router and a copy without, which is what VTU recommends. - That means that src/setup.js can no longer import src/plugins.js. Instead, src/plugins.js is imported in src/main.js. - For some components, these changes result in a natural sequence of tests: tests first use mount() to test the behavior of the component before a request, then use mockHttp() to test behavior before a successful response, then use load() to test a route change and other behavior after a successful response. For some components (for example, AccountLogin), I reordered tests to match this sequence. - load() will now inject the router only if it is mounting the root component (App). In other cases, it stubs <router-link> and mocks $route. - The documentation for mockHttp() and load() describes some of this in more detail, including guidelines around which utility function to use in different cases. - One case where mocking $route doesn't work is if an async component uses $route: see vuejs/vue-test-utils#1486. This was an issue for ProjectSubmissionOptions, so it is no longer loaded async. I had actually already wanted not to load it async: it's not a large component, and it may be needed soon after the page renders. I think I set it up to load async because multiple components import it, but I don't think I realized at the time that webpack will automatically split out a large shared component into its own .js file. - test/index.js used to contain some complexity related to the router, but with these changes in place, it felt like the right time to remove the previous workaround. I was able to simplify things by having the router use abstract mode in testing. This change also sped up tests significantly. - Previously, one of the tests of AccountClaim would fail intermittently depending on how quickly the Password async component loaded. After these changes, the test failed more often (maybe because some of the tests before it are now faster?), so I updated the test to wait for the component to load. This involved a small change to FormGroup. - There are benefits to mocking/stubbing for Vue Router, but there seem to be fewer benefits to doing so for our other plugins, Vuex and Vue I18n. The router contains a fair amount of logic and implements some of that behavior (some of which can be async) as soon as it is injected into a component. However, the same is not true of Vuex or Vue I18n. - I'm not sure whether avoriaz created a parent component when mounting, but Vue Test Utils seems to. We destroy the parent component after each test. We used to destroy the component before removing it from the DOM, but we now do those in the reverse order. That matches what avoriaz and VTU do, and I don't see any obvious issues with that approach. (We introduced the previous logic in 304e5db, maybe because Modal at the time used a ref in its beforeDestroy hook. However, that is no longer the case.) - VTU automatically sets Vue.config.productionTip and Vue.config.devtools to `false`. This removes a message about Vue devtools that was previously shown in testing. Since VTU now sets productionTip, I have moved that configuration from src/setup.js to src/main.js. - VTU wrappers have an isVisible() method, but I think we should continue using our visible() and hidden() assertions. One reason for that is that both assertions can be used to test style-based visibility. Also, the hidden() assertion is more specific than not.visible(). - Make other improvements to testing, including: - Remove deprecated functions and properties: - Replace mockRoute() with load() or mockHttp(). - Replace mountAndMark() with mount(). - Replace mockHttp().standardButton() with mockHttp().testStandardButton(). - Replace testData.administrators with testData.standardUsers. - Remove the disabled() assertion. - Remove String.prototype.iTrim(). - Use mockHttp().testModalToggles() in more tests, and call it with a single argument consistently. - Mount the root component (App) in fewer tests. - I initially made this change in too many cases. I've added a check to load() to help prevent that going forward. - Add an id or class attribute to certain elements to make it easier to select elements in testing. In other cases, shorten an existing class name. - Move tests: - From UserList to router.spec.js - From ProjectRow to ProjectIntroduction - Remove unneeded tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
We currently use avoriaz for our testing, but Vue Test Utils seems to be where the community is heading. Vue Test Utils is still in beta, but it has been under development for a while, which makes me think that it should be fine to move to it.
The text was updated successfully, but these errors were encountered: