Fix file path issues #833
Fix file path issues #833
Conversation
drewpc
force-pushed
the
patch-fix-file-path-issues
branch
from
42b5cc9
to
c0ef934
Compare
| // add a wildcard onto the end if no file extension or wildcard | ||
| // currently present | ||
| let specGlob = "*[Ss]pec.js"; | ||
| if (!specGlob.endsWith(".js") && !specGlob.endsWith("*")) { |
There was a problem hiding this comment.
I don't understand this at all -- you just declared specGlob? How can it have changed already?
There was a problem hiding this comment.
I just copied that from the other gulp files where jasmine was being used.
There was a problem hiding this comment.
The other usage allows a CLI flag on the tests themselves to override specs (e.g. gulp test --specs Foo) and then appends a * to turn it into a glob prefix match if it isn't one already. I think here we could get rid of the if, unless we plan on supporting the CLI flag later.
There was a problem hiding this comment.
Oh, just realized this was left outstanding.
@drewpc @doug-wade Should we file a followup ticket for this?
There was a problem hiding this comment.
Talked with @drewpc. Turns out this did get resolved. 😌
| let cwd = process.cwd(), | ||
| lookupResult; | ||
|
|
||
| if (process.env.NODE_ENV === '__react-server-cli-unit-test__') { // eslint-disable-line no-process-env |
There was a problem hiding this comment.
This doesn't seem right.
There was a problem hiding this comment.
I agree. Got any other ideas on how to bypass callerDependency when running unit tests inside the same directory?
There was a problem hiding this comment.
I'm not sure if it's better in the overall scheme of things (mockery is black magic), but could we use mockery to intercept requires of callerDependency from the spec itself? At least that way the complexity is in the spec, rather than here.
|
This also fixes #534. I tested it specifically on Windows and it's working like a charm. |
| @@ -24,7 +37,11 @@ gulp.task("eslint", [], () => { | |||
| }); | |||
|
|
|||
| // there are no tests for this project :( | |||
There was a problem hiding this comment.
I think this comment can go away now :)
| let cwd = process.cwd(), | ||
| lookupResult; | ||
|
|
||
| if (process.env.NODE_ENV === '__react-server-cli-unit-test__') { // eslint-disable-line no-process-env |
There was a problem hiding this comment.
I'm not sure if it's better in the overall scheme of things (mockery is black magic), but could we use mockery to intercept requires of callerDependency from the spec itself? At least that way the complexity is in the spec, rather than here.
| // add a wildcard onto the end if no file extension or wildcard | ||
| // currently present | ||
| let specGlob = "*[Ss]pec.js"; | ||
| if (!specGlob.endsWith(".js") && !specGlob.endsWith("*")) { |
There was a problem hiding this comment.
The other usage allows a CLI flag on the tests themselves to override specs (e.g. gulp test --specs Foo) and then appends a * to turn it into a glob prefix match if it isn't one already. I think here we could get rid of the if, unless we plan on supporting the CLI flag later.
|
@roblg I can try to refactor that using a mock. That would be much cleaner, you're right. |
|
Finished refactoring the PR to use a mock of |
…log instrumentation. Update gulpfile and test spec.
…Dependency and add mockery to handle properly requiring the mock instead of the original callerDependency.
drewpc
force-pushed
the
patch-fix-file-path-issues
branch
from
1cb76c2
to
c749a76
Compare
|
|
||
| const fileData = mockFs.toString(); | ||
| filePathRegexStrings.map((regex) => { | ||
| expect(fileData).toMatch(regex.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&")); |
There was a problem hiding this comment.
Wow, that's... quite a regex! 😳
| let routesOutput = []; | ||
|
|
||
| const coreMiddleware = require.resolve("react-server-core-middleware"); | ||
| const coreMiddleware = JSON.stringify(require.resolve("react-server-core-middleware")); |
Fixes #832 and includes tests! The fix was easy...the tests were hard to develop.