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

Adds Jest, Vue test utils, and a test suite for the VanillaVueInput component #33

Merged
merged 7 commits into from
Dec 10, 2019

Conversation

jrgriffiniii
Copy link
Contributor

Advances #26 with the following:

  • Introduces Jest and the Vue test utils packages for testing Vue components
  • Introduces a Jest configuration file for generating test
    coverage reports and handling .vue templates
  • Adds a test suite for the VanillaVueInput component

@jrgriffiniii
Copy link
Contributor Author

Unfortunately vuejs/vue-jest#160 (comment) was a problem, otherwise one would not need to include "babel-core": "^7.0.0-bridge.0".

it('renders the correct markup', () => {
expect(wrapper.html()).toContain("<form>\n <fieldset><input type=\"text\" placeholder=\"Type a value here...\"></fieldset>\n <p>Value: </p>\n</form>")
})
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jrgriffiniii until we get to #30 , mind installing a Prettier extension in your IDE, and set it to default settings? Otherwise if we're both working on similar files the diffs are going to be a of of quotation marks and semicolons:)

Copy link
Contributor

@adamjarling adamjarling Dec 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, just a thought, what about renaming the file using kebab case to establish a pattern for .js files? I know it's a small thing and no big deal, but I've found switching between lots of different projects, just quick eye glancing files, when I see kebab case I know its JS, and snake case (is that what it's called?) I know it's generally Ruby, Elixir, etc.

@@ -3,7 +3,8 @@
"license": "MIT",
"scripts": {
"deploy": "webpack --mode production",
"watch": "webpack --mode development --watch"
"watch": "webpack --mode development --watch",
"test": "jest --config ./js/test/jest.conf.js --coverage"
Copy link
Contributor

@adamjarling adamjarling Dec 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if there's a way to reference this file path ./js/test/jest.conf.js outside of the command? Jest might by default look for certain config file(s) where we could declare it? If not, no big deal.

Copy link
Contributor

@adamjarling adamjarling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs just as expected. Nice work. Feel free to take or leave the comments.

@jrgriffiniii jrgriffiniii changed the title Adds Jest, Vue test utils, and a test suite for the VanillaVueInput component [WIP] Adds Jest, Vue test utils, and a test suite for the VanillaVueInput component Dec 9, 2019
@jrgriffiniii jrgriffiniii changed the title [WIP] Adds Jest, Vue test utils, and a test suite for the VanillaVueInput component Adds Jest, Vue test utils, and a test suite for the VanillaVueInput component Dec 9, 2019
Copy link
Contributor

@adamjarling adamjarling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrgriffiniii This looks great. I've been building on top of it... do you want to merge this as-is, or should I add commits on top, then merge the whole feature branch in at once?

@jrgriffiniii
Copy link
Contributor Author

I would support merging this as-is please. This would permit you to then continue your progress based off of the master branch and segregate more commits between separate branches.

Copy link
Contributor

@adamjarling adamjarling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@

"!<rootDir>/js/components/_components.js",
"!<rootDir>/js/components/_lux-elements-used.js",
"!<rootDir>/node_modules/**"
]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrgriffiniii I was probably unclear in a previous PR comment, but I liked how you had these jest configs in the previous jest config file. What I meant to say was that we might not (not positive) need to declare the path to jest.config.js or whatever the file is called, in the test npm command.

Either way, no big deal. Tested, and locally runs as expected. Again, nice work. I'll save my work on top of this and do a PR in the near future to expand some testing scenarios with embedded components and other more advanced scenarios which will pop up in a future app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for that, I have restored this with 92ca869

@jrgriffiniii jrgriffiniii force-pushed the issues-26-jrgriffiniii-vanilla-vue-input-test branch from c5c9aa0 to 074ea91 Compare December 9, 2019 16:31
@jrgriffiniii
Copy link
Contributor Author

The tests should fail given that I am specifying a required threshold of 100% test coverage for the Jest test suites with 92ca869#diff-8b1e675a9fa0a68d9c4f68b81f920a74R20

jrgriffiniii and others added 7 commits December 9, 2019 16:08
components; Introduces a Jest configuration file for generating test
coverage reports and handling .vue templates; Adds a test suite for the
VanillaVueInput component

Co-authored-by: Adam Arling <adam.arling@northwestern.edu>
…e Jest configuration into the package.json file
@jrgriffiniii jrgriffiniii force-pushed the issues-26-jrgriffiniii-vanilla-vue-input-test branch from 304e0d5 to 317d113 Compare December 9, 2019 21:08
@jrgriffiniii jrgriffiniii merged commit a2370c9 into master Dec 10, 2019
@jrgriffiniii jrgriffiniii deleted the issues-26-jrgriffiniii-vanilla-vue-input-test branch December 10, 2019 06:16
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.

2 participants