-
Notifications
You must be signed in to change notification settings - Fork 34
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
Set up test infrastructure #56
Conversation
package.json
Outdated
"downshift": "^1.22.5", | ||
"@department-of-veterans-affairs/react-jsonschema-form": "^1.0.0", | ||
"eslint-plugin-import": "^2.7.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering why these two eslint deps are not in devDeps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, short answer is I've been transferring a lot of the set up from the Vets.gov codebase and that's how these packages were installed, as regular dependencies. Looking into them, I think it's possible to just have them installed as devDependencies, so I'll change that for now and we can always switch back if for some reason an issue arises with that in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
package.json
Outdated
}, | ||
"dependencies": { | ||
"babel-plugin-dynamic-import-node": "^1.2.0", | ||
"blob-polyfill": "^2.0.20171115", | ||
"choma": "^1.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be in devDeps?
Not having run the test, I can't confirm the workings of the PR, but everything else looks fine to me. I'd say if the test runs (and you can make it fail) and the linting runs (and you can make it warn you), then |
Can confirm the linter catches errors because there are currently many errors. However, I'm going to be doing a new port of the files from vets.gov, so soon those will all be fixed. Test also runs and passes (when I change the environment file, which will also be changed in the new port). Going to merge! |
thank you @jordangov :) |
Title line template: Test infrastructure set up
Description
Install packages and set up configuration necessary for unit tests and linting to run properly. Include one sample test to ensure things are working, but will port the entire test suite in a separate PR.
Additional information
I did not not include functionality for e2e tests yet because I'm not sure what kinds of e2e tests we'll want to create for the library. I think we should address that as a separate ticket.