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

Use vue-test-utils for testing #335

Merged
merged 2 commits into from
Dec 5, 2018

Conversation

josephting
Copy link
Contributor

Description

  1. Added vue-test-utils to package.json
  2. Rewritten all tests to use vue-test-utils retaining the original method used in each of the tests.

I have noticed some tests aren't doing anything while other does not generate exact same snapshot but those fixes will come later.

Motivation and Context

Effort being placed in so that work on increasing code coverage can eventually be carried out for the project.

How Has This Been Tested?

  1. All the assertion have been negated i.e. adding .not or removing .not to test if they can fail
  2. precommit passed as before

Screenshots (if appropriate):

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have included a vue-play example (if this is a new feature)

Todo:

  • Fix should apply 200px carousel width when element has 200px width test case (currently, it does not fail even after adding .not to the assertion)

@coveralls
Copy link

coveralls commented Dec 3, 2018

Coverage Status

Coverage increased (+0.6%) to 67.834% when pulling 7cd7e57 on josephting:feat/test-utils into 87d8dfc on SSENSE:v0.17.0.

@quinnlangille
Copy link
Member

Great work @josephting. The PR review form really show us how little tests we have, so super glad you're taking on this initiative. I'll wait for a response from @ashleysimpson on your comment re: snapshots, but otherwise looks good :octocat:

@josephting
Copy link
Contributor Author

Thank you, @quinnlangille. This PR is ready to be merged now. Then, we can have #337 merged as well.

@quinnlangille
Copy link
Member

Great! Will merge into the working branch now, really great work @josephting 🚀

@quinnlangille quinnlangille merged commit 904e069 into SSENSE:v0.17.0 Dec 5, 2018
@josephting josephting deleted the feat/test-utils branch December 6, 2018 00:11
quinnlangille pushed a commit that referenced this pull request Dec 19, 2018
quinnlangille pushed a commit that referenced this pull request Dec 27, 2018
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.

4 participants