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
Passing defaultOptions to testem to prevent the cwd and config_dir set in testem.js from being overridden by ember-cli #7700
Conversation
tests/unit/tasks/test-server-test.js
Outdated
@@ -45,6 +45,46 @@ describe('test server', function() { | |||
watcher.emit('change'); | |||
}); | |||
|
|||
it('transforms and sets defaultOptions in testem and invokes testem properly', function(done) { |
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.
i would prefer if we just use promises here, rather then a mix of done
and promises. The promise mode for mocha will nicely handle errors for us.
lib/tasks/test-server.js
Outdated
return new Promise((resolve, reject) => { | ||
task.testem.startDev(task.testemOptions(options), (exitCode, error) => { | ||
if (task.testem.setDefaultOptions) { | ||
task.testem.setDefaultOptions(task.testemOptions(options)); |
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.
this looks good
lib/tasks/test-server.js
Outdated
if (task.testem.setDefaultOptions) { | ||
task.testem.setDefaultOptions(task.testemOptions(options)); | ||
} else { | ||
// To make it backward compatibile with older versions of testem (2.0.0 and lower) |
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.
I don't think we need this branch, as ember-cli will need to bump its minimum testem version to one that supports this feature
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.
pending @johanneswuerbach releasing testem for us.
I’m currently on vacation, but I’ll cut a new version once I’m back in 2 days |
Fixing the issue in #7728 and closing this PR. |
When ember-cli invokes testem it provides defaultOptions for testem to use as a fallback option. But currently, the fallback option from ember-cli is used as progOptions in testem. And since progOptions has higher priority than fileOptions, the cwd being set in the testem.js file of an ember app is being overridden by ember-cli's fallbackOption.
Solution is to add defaultOptions as a sibling to progOptions and fileOptions in testem. And ember-cli can set defaultOptions invoke testem.
Related work in testem is in the below PR.
testem/testem#1219