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

Vue unit test cleanup, removing jquery, mocking ajax calls #10904

Merged
merged 12 commits into from
Dec 18, 2020

Conversation

Nerdinacan
Copy link
Contributor

@Nerdinacan Nerdinacan commented Dec 11, 2020

There's a lot of client-side unit tests that die or become erratic due to unnecessary 3rd party library usage or unmocked ajax calls.

Jest doesn't run in a browser, it runs in node, and code that expects to talk to native browser objects (like document or XHR) either dies or runs without response. This is particularly problematic when jQuery is used. I'll be removing that first, then moving on to all the multitudes of unmocked ajax calls.

Notable changes:

  1. Removed old babel rewire mocking mechanism in favor of jest mocking tools
  2. Removed global Vue instances in unit tests. This is normally pretty innocuous, but as more people hang plugins and mixins off of the Vue instance, this can create problems that extend beyond the scope of a single test. Especially problematic are plugins or mixins that reference legacy code that either includes jQuery or fires instant ajax calls upon include, which includes most of the legacy backbone code, and some of the new Vuex initialization use-cases.
  3. Removed usage of a jest utility mounter that referenced a non-existent document object.
  4. Replaced mounts with shallowMounts in Vue unit tests when possible. In general it's a good idea to try a shallowMount first and then if you REALLY need to, go ahead and do a full mount. Also ask yourself if what you're writing is really a unit test if you require a full tree mount. Perhaps what you are trying to do would be better accomplished with an integration test.

@Nerdinacan Nerdinacan self-assigned this Dec 11, 2020
@bgruening
Copy link
Member

Oh yeah, unleash the @Nerdinacan!

@Nerdinacan Nerdinacan force-pushed the test_cleanup branch 3 times, most recently from 08190b6 to 4dc16c4 Compare December 17, 2020 03:14
@Nerdinacan Nerdinacan marked this pull request as ready for review December 17, 2020 06:14
@Nerdinacan
Copy link
Contributor Author

Our client-side unit tests still need a lot of attention, but they'll probably have to be addressed one at a time. These global changes are ready now, however.

jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/src/app/__mocks__/index.js
    * <rootDir>/src/components/History/caching/__mocks__/index.js
@Nerdinacan Nerdinacan changed the title Test cleanup, removing jquery, mocking ajax calls Vue unit test cleanup, removing jquery, mocking ajax calls Dec 17, 2020
@Nerdinacan
Copy link
Contributor Author

@dannon
Oooh thanks for getting rid of that browser compatibility notice. The error message tells me what to do to fix it and yet I have stared at it for a year without updating it.

@dannon
Copy link
Member

dannon commented Dec 17, 2020

@Nerdinacan Hah, yeah, same -- I just never got around to it and now I'm in "silence all the warnings" mode.

@Nerdinacan
Copy link
Contributor Author

I like what you've done with the place.

@dannon dannon merged commit b7cb98c into galaxyproject:dev Dec 18, 2020
@github-actions
Copy link

This PR was merged without a "kind/" label, please correct.

@Nerdinacan Nerdinacan deleted the test_cleanup branch December 18, 2020 19:00
@jmchilton jmchilton added this to the 21.01 milestone Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants