Skip to content
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

render calls in before/beforeEach aren't converted #32

Closed
apellerano-pw opened this issue Jun 25, 2018 · 11 comments · Fixed by #36
Closed

render calls in before/beforeEach aren't converted #32

apellerano-pw opened this issue Jun 25, 2018 · 11 comments · Fixed by #36

Comments

@apellerano-pw
Copy link

apellerano-pw commented Jun 25, 2018

Sometimes when tests have similar setups we use a beforeEach to call render. The codemod does a good job converting it usages to async functions but misses functions passed to other mocha hooks

@ghost
Copy link

ghost commented Jun 29, 2018

This sounds like the same issue in: #26. would you mind confirming? I'm picking the codemod up again today to see if I can get it rewriting properly.

@apellerano-pw
Copy link
Author

Yes it seems like the same issue to me. Thanks for making a fix!

@ghost
Copy link

ghost commented Jun 29, 2018

cool thanks! no problem. I think I may have fixed the issue here: https://github.com/efx/ember-mocha-codemods/tree/fix-14
If you have time, you could try that branch against your codebase and let me know if you have any issues.

@apellerano-pw
Copy link
Author

apellerano-pw commented Jun 29, 2018

I ran it on a file, here is the before and after.

While it did convert the render to the one imported from @ember/test-helpers, it didn't add async or await to the code so I don't think it would be a sufficient fix for my case

beforeEach(function () {
  this.render(hbs`{{my-component
    isActiveCollection=true
    collection=collection
  }}`);
});
beforeEach(function () {
  render(hbs`{{my-component
    isActiveCollection=true
    collection=collection
  }}`);
});

@apellerano-pw
Copy link
Author

apellerano-pw commented Jun 29, 2018

Here's an input file

import { expect } from 'chai';
import {
  beforeEach,
  context,
  describe,
  it,
} from 'mocha';
import { setupComponentTest } from 'ember-mocha';
import hbs from 'htmlbars-inline-precompile';

describe('Integration | Component | my-component', function () {
  setupComponentTest('my-component', {
    integration: true,
  });

  context('when in some situation or another', function () {
    beforeEach(function () {
      this.render(hbs`{{my-component}}`);
    });

    it('should do the thing', function () {
      expect(true).to.be.true;
    });
  });
});

I ran it thru your fork with
jscodeshift -t https://raw.githubusercontent.com/efx/ember-mocha-codemods/fix-14/fourteen-testing-api.js fake-test.js

@ghost
Copy link

ghost commented Jun 29, 2018

sweet, thanks for checking. I have found a few other bugs too. I will add this snippet above as a test too.

@ghost
Copy link

ghost commented Jun 29, 2018

@apellerano-pw I just updated the branch with a passing test accounting for the context hook. If it runs ok on your codebase, I'll open the PR.

@apellerano-pw
Copy link
Author

ayyyy it worked! 🎉

@apellerano-pw
Copy link
Author

Oh, if the problem was that the context() alias wasn't working, what about the specify() alias for it()? I didn't see that in a quick search of the codemod either. This might be worthy of its own ticket

https://mochajs.org/#bdd

context() is just an alias for describe(), and behaves the same way; it just provides a way to keep tests easier to read and organized. Similarly, specify() is an alias for it().

@ghost
Copy link

ghost commented Jun 29, 2018

good point. I think a new ticket is best so we can keep the PR's smaller.

@apellerano-pw
Copy link
Author

I made #37 for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant