-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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 precompiled version of jasmine-only #579
Conversation
var jasmineFileContent = | ||
fs.readFileSync(require.resolve(JASMINE_PATH), 'utf8'); | ||
const JASMINE_PATH = require.resolve('../../vendor/jasmine/jasmine-1.3.0'); | ||
const jasmineFileContent = fs.readFileSync(JASMINE_PATH, 'utf8'); |
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.
Yeah we only need to require.resolve
once.
24a6537
to
8efc492
Compare
Ship a static, precompiled version of jasmine-only to drop the coffee-script requirement. - Add the precompiled version if jasmine-only to `vendor/jasmine-only/` - Update `src/jasmineTestRunner/jasmineTestRunner.js` to always use the shipped version - Drop coffee-script from `package.json`
@facebook-github-bot import |
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/108676296160596/int_phab to review. |
@facebook-github-bot shipit |
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/108676296160596/int_phab to review. |
6001e8e
I cannot do that @cpojer, because @clentfort does not have an active CLA on file. |
1 similar comment
I cannot do that @cpojer, because @clentfort does not have an active CLA on file. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Ship a static, precompiled version of jasmine-only by @davemo to drop the coffee-script requirement. Fixes #578.
Not sure if we need to update the headers of the precompiled file to include the license. FWIW According to the package.json its under the Apache License 2.0.
Test plan:
describe
-block and two tests: One created withit
and one withit.only
, let both tests print to STDOUT, verify only the contents of the test created withit.only
are printed to STDOUT.describe
- and onedescribe.only
-block, in both blocks print to STDOUT, verify that only thedescribe.only
-block prints to STDOUT.