Add tests for server side rendering of Soy and JSX components #269
Conversation
robframpton
force-pushed
the
iso_test
branch
4 times, most recently
from
9539473
to
7d9fea0
Compare
robframpton
changed the title
.babelrc
Outdated
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "presets": ["es2015"] | |||
There was a problem hiding this comment.
I think es2015 has been deprecated for a while in favor of env... could we try using it instead?
| @@ -102,6 +102,12 @@ gulp.task('soy', function() { | |||
| .pipe(gulp.dest('packages/metal-soy/test')); | |||
| }); | |||
|
|
|||
| gulp.task('soy:isomorphic', function() { | |||
There was a problem hiding this comment.
Do we need gulp for this? Since we're invoking it from npm run can't we simply call metalsoy?
There was a problem hiding this comment.
That's what I did at first, unfortunately this repo still uses an older version of metal-tools-soy, and updating it to the version that included the cli caused some issues.
Once this is merged I'd like to update it, resolve the issues, and remove whatever gulp tasks we can.
| "metal-component": "^2.13.2", | ||
| "metal-jsx": "^2.13.2", | ||
| "metal-soy": "^2.13.2" | ||
| } |
There was a problem hiding this comment.
Aren't we missing devDependencies here? (I know it's probably picked by lerna, but it feels weird, not sure if that's how it's supposed to be :) )
There was a problem hiding this comment.
Well, since the tests aren't actually run from this package, it will only add bloat to lerna bootstrap since the devDependencies in this package wouldn't be used. I can see why it looks a little confusing though, do you want to add mocha and the other test dependencies here?
There was a problem hiding this comment.
Sounds good, if that's what we're also doing in other projects... we need to converge all the monorepos' infrastructure :)
| import Component from 'metal-component'; | ||
| import MyComponent from './fixtures/MyComponent'; | ||
| import MyJSXComponent from './fixtures/MyJSXComponent'; | ||
| import { assert } from 'chai'; |
There was a problem hiding this comment.
Wouldn't we rather use jest snapshots here? Since we're already using it in other places and we won't be testing this on browsers... that way we have the same test dep everywhere.
There was a problem hiding this comment.
Do you think we can make that improvement down the line? There are a lot of places in this repo that we could be using jest snapshots, it would be nice to do that migration all at once under a separate issue.
|
This LGTM, then, just waiting on #274 to retest. |
|
Waiting on #274 to rebase this branch, as it will cause more conflicts. |
|
Awesome!!! All is green!!! 🎉 As I mentioned, my only concern right now is that testing jumps from 6-8 minutes to 13-15 on this PR, so I'd like to see if we can figure out why. Could you take a look? |
|
It's retesting right now, but I suspect it's because Edge is hanging for some reason. |
|
@jbalsas Edge didn't hang in the retest, and the time was significantly shorter. |
|
Merging then!! 🎉 |
Note: the tests on this branch will fail until the following branches are merged.
#264
#265
#266
This branch adds the private
metal-isomorphicpackage for the purpose of testing server side rendering of components. It's only intention is to add tests that can leverage from the lerna environment, which is why theprivateflag has been set totrue.