-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add tests #33
Comments
|
First big problem I ran into when creating the test harness was ...But then I ran into a bunch of issues with configuration:
P.S. It turns out upgrading to Babel 7 in this specific repo shouldn't be too difficult as I think the only proposal we're using is
|
Oh yea, so I decided to use Now, of course, looking at all the above stuff, there still ended up being quite a bit to configure for the specific environment here, so that kinda sucked 😕 .
|
And of course, just as I got the test harness to finally work, it bugged out on We'll see if I might need some later versions of Oh also worth to note is that |
- add 1 simple test for existence of canvas and instance methods - change test script to use jest, and move prev test to test:pub - change CI to run test and test:pub - configure jest and enzyme for testing usage - make jest importable too - add .babelrc to configure babel-jest - can't use .babelrc.js as babel-jest@23 doesn't support it - jestjs/jest#5324 - can't use .babelrc for babel-loader because babel-loader@6 has some bugs with it and @7 doesn't support webpack@1 - babel/babel-loader#552 - use jest instead of ava mainly because there's less to configure / install (directly at least, still a lot indirectly), it's popular / well-supported in React community, and supports configuration outside of package.json - see #33 for some more details (deps): add jest, enzyme, and supporting deps to devDeps - add babel-jest@23 for Babel 6 support - and configure it for .js files due to a jest bug - add canvas-prebuilt@1 to support jest's jsdom v11 - canvas is used by jsdom for, well, canvas interactions - add enzyme-adapter-react-16 to configure Enzyme with React 16 - upgrade to React 16 in devDeps bc adapter-react-15 requires react-test-renderer and no drawbacks in upgrading
- add 1 simple test for existence of canvas and instance methods - change test script to use jest, and move prev test to test:pub - change CI to run test and test:pub - configure jest and enzyme for testing usage - make jest importable too - add .babelrc to configure babel-jest - can't use .babelrc.js as babel-jest@23 doesn't support it - jestjs/jest#5324 - can't use .babelrc for babel-loader because babel-loader@6 has some bugs with it and @7 doesn't support webpack@1 - babel/babel-loader#552 - use jest instead of ava mainly because there's less to configure / install (directly at least, still a lot indirectly), it's popular / well-supported in React community, and supports configuration outside of package.json - see #33 for some more details (deps): add jest, enzyme, and supporting deps to devDeps - add babel-jest@23 for Babel 6 support - and configure it for .js files due to a jest bug - add canvas-prebuilt@1 to support jest's jsdom v11 - canvas is used by jsdom for, well, canvas interactions - add enzyme-adapter-react-16 to configure Enzyme with React 16 - upgrade to React 16 in devDeps bc adapter-react-15 requires react-test-renderer and no drawbacks in upgrading
So I was getting some errors with dataURL fixtures when I realized the above^. But seems like those error isn't because of the I also had some trouble with trimming the canvas, getting wrapper methods and
|
- add 1 simple test for existence of canvas and instance methods - (pkg): test code shouldn't be in the package, just source - currently preferring to keep source and tests co-located - change test script to use jest, and move prev test to test:pub - change CI to run test and test:pub - configure jest and enzyme for testing usage - make jest importable too - add .babelrc to configure babel-jest - can't use .babelrc.js as babel-jest@23 doesn't support it - jestjs/jest#5324 - can't use .babelrc for babel-loader because babel-loader@6 has some bugs with it and @7 doesn't support webpack@1 - babel/babel-loader#552 - use jest instead of ava mainly because there's less to configure / install (directly at least, still a lot indirectly), it's popular / well-supported in React community, and supports configuration outside of package.json - see #33 for some more details (deps): add jest, enzyme, and supporting deps to devDeps - add babel-jest@23 for Babel 6 support - and configure it for .js files due to a jest bug - add canvas-prebuilt@1 to support jest's jsdom v11 - canvas is used by jsdom for, well, canvas interactions - add enzyme-adapter-react-16 to configure Enzyme with React 16 - upgrade to React 16 in devDeps bc adapter-react-15 requires react-test-renderer and no drawbacks in upgrading
- add 1 simple test for existence of canvas and instance methods - (pkg): test code shouldn't be in the package, just source - currently preferring to keep source and tests co-located - change test script to use jest, and move prev test to test:pub - (ci): change CI to run test and test:pub - configure jest and enzyme for testing usage - make jest importable too - add .babelrc to configure babel-jest - can't use .babelrc.js as babel-jest@23 doesn't support it - jestjs/jest#5324 - can't use .babelrc for babel-loader because babel-loader@6 has some bugs with it and @7 doesn't support webpack@1 - babel/babel-loader#552 - use jest instead of ava mainly because there's less to configure / install (directly at least, still a lot indirectly), it's popular / well-supported in React community, and supports configuration outside of package.json - see #33 for some more details (deps): add jest, enzyme, and supporting deps to devDeps - add babel-jest@23 for Babel 6 support - and configure it for .js files due to a jest bug - add canvas-prebuilt@1 to support jest's jsdom v11 - canvas is used by jsdom for, well, canvas interactions - add enzyme-adapter-react-16 to configure Enzyme with React 16 - upgrade to React 16 in devDeps bc adapter-react-15 requires react-test-renderer and no drawbacks in upgrading
Got to 100% line/statement coverage last night, some notes:
Also changed some code style things:
|
Found another weird issue that I figured out which was whether Enzyme's The callback was indeed added because it was (sometimes?) working async per this issue. But the currently last comment on that issue says that it was updated to work synchronously in v3.4.3 for ease of testing. But v3.4.3 changelog doesn't explicitly say that... Real confusing, but I just rewrote my code to use it sync everywhere then as it's more readable and works fine that way 🤷♂ |
Oh and forgot to mention that I looked into Code Coverage reporting providers and set-up CodeCov as it seemed to very popular and easy (and no need for tokens for public repos w/ public CI) with a one-line bash uploader. Can view the reporting here: https://codecov.io/gh/agilgur5/react-signature-canvas/ |
- add 1 simple test for existence of canvas and instance methods - (pkg): test code shouldn't be in the package, just source - currently preferring to keep source and tests co-located - change test script to use jest, and move prev test to test:pub - (ci): change CI to run test and test:pub - configure jest and enzyme for testing usage - make jest importable too - add .babelrc to configure babel-jest - can't use .babelrc.js as babel-jest@23 doesn't support it - jestjs/jest#5324 - can't use .babelrc for babel-loader because babel-loader@6 has some bugs with it and @7 doesn't support webpack@1 - babel/babel-loader#552 - use jest instead of ava mainly because there's less to configure / install (directly at least, still a lot indirectly), it's popular / well-supported in React community, and supports configuration outside of package.json - see #33 for some more details (deps): add jest, enzyme, and supporting deps to devDeps - add babel-jest@23 for Babel 6 support - and configure it for .js files due to a jest bug - add canvas-prebuilt@1 to support jest's jsdom v11 - canvas is used by jsdom for, well, canvas interactions - add enzyme-adapter-react-16 to configure Enzyme with React 16 - upgrade to React 16 in devDeps bc adapter-react-15 requires react-test-renderer and no drawbacks in upgrading
Place for notes as I'm implementing tests on the
tests
branch.Some initial notes:
enzyme
for component testing as we actually do need the ref in this library's case to test instance methods, so can't usereact-testing-library
. We also do need to test actually mounting to the DOM (due tocanvas
usage), soreact-test-renderer
doesn't really suffice either.canvas
like thecanvas
package orcanvas-prebuilt
, but apparently need fulljsdom
support for enzyme (https://airbnb.io/enzyme/docs/guides/jsdom.html). The set-up forjsdom
forenzyme
however seems equivalent to whatbrowser-env
does, so that can be a replacement.The text was updated successfully, but these errors were encountered: