-
Notifications
You must be signed in to change notification settings - Fork 25
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
Shallow-render should now support both Jest & Jasmine #100
Conversation
Great work! I think you covered things nicely and I like that the framework is auto-detected. I think it's fine to declare jest/jasmine as I think that since the user should be interacting with jest/jasmine directly in their tests so we should consider keeping the What do you think about converting the public static property to a singleton module that can be used internally like: import { testFramework } from './lib/test-framework';
testFramework.spyOn(); This could be accomplished by converting the // I've taken some liberties in reducing the detection logic you used in `getAvailableTestFramework`
export const testFramework = typeof jest === 'undefined'
? new JasmineFramework()
: new JestFramework(); I think we could probably put out a pre-release NPM package that we can use to test out in a sample project with Jest. Thanks again for the PR, glad to have your support! |
Thanks for reviewing my PR, I've extracted the public class property to a singleton module as you suggested. I'd be happy to keep contributing to this project 🎉 |
Cool, gonna test our your changes. I noticed some weirdness in the TSC output with an import, I think I have a fix and I'll submit a PR to your branch when I'm done. Once this is all merged, I'll add you to the contributors list! |
Looks like you already resolved the import issue in your latest change. This tests well, I created an angular 6 project and tests ran with Karma and Jest! |
Awesome guys. I'll test this when I get some time as well. |
Should fix #99
Couple of points:
There's a note about being type safe with public contracts - I wanted to add, that the implementations for wrapping jest & jasmine are now using
any
in more places than I'd like. Since it's used internally, I could settle with it, but in the current implementation this is exposed as a public static property on theShallow
class, so it's available for consumers of this library. I wanted to avoid installing packages that'd help with the typings, but I can see that as an option.Test coverage - I have added exactly 0 tests for these changes, since I've found it difficult to test for global objects/namespaces. Also, in the library itself, there's nothing at the moment ensuring that tests run with jest (I did test it on the example that I've attached in Adding jest support #99 though). The only way I can think of is to set up sample projects that use different testing frameworks.
I think I explicitly went against "No funny business" with the way I'm trying to determine if jest is available or not, although I'd let you be the judge of that. I've checked
ts-mockery
(as suggested in Adding jest support #99 ) and that package seems to accept a configuration for which test runner to use. I'm being selfish, and I know I wouldn't want to set that up withshallow-render
if it can be avoided, but not sure if the current implementation is better than allowing external configuration.Tried to come up with an implementation that's somewhat in line with https://github.com/getsaf/shallow-render/projects/1#card-18903827 - not sure how well I did on that