-
Notifications
You must be signed in to change notification settings - Fork 6
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
transform route and controller tests #70
Conversation
|
||
describe('LoadingIndexRoute', function() { | ||
setupTest('route:loading/index', { | ||
// needs: ['service:ajax'] |
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.
Does this comment serve a purpose to the test? Like, is this to ensure comments are also removed?
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.
@rondale-sc indeed. I wanted to have a sample of what you might find in an older ember-mocha
codebase. Not strictly necessarily, but fulfilling a goal of mine to test against real world snippets.
function findOtherUnitTestSetup(nodePath) { | ||
let hasSetup = j(nodePath).find(j.ExpressionStatement, { expression: { callee: { name: 'setupTest' } } } ) | ||
let isRouteTest = hasSetup | ||
&& j.ExpressionStatement.check(nodePath) |
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.
Not something to hold the PR up for, but it might be nice having a little comment to the right of each line here to let future travelers know more easily what is going on here since this seems quite involved.
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.
Completely agreed! Hearty advice for most codemod authoring :).
This looks great! Thanks! 🍻 |
fix #69. Also fixes a bug with nested lifecycle hookds causing subjectContainerKey to be undefined.
LGTM - Will release on monday since I don't have npm creds on my new machine yet. |
fix #69. Also fixes a bug with nested lifecycle hooks causing
subjectContainerKey
to be undefined.I tested these transforms against our application running ember-mocha@0.14. The transform work, but I am seeing a bunch of errors I can detail elsewhere.