-
Notifications
You must be signed in to change notification settings - Fork 99
add initial ember-cli-addon-tests #173
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
Conversation
Looks like the 5 minute timeout was too short for Appveyor. :( |
591876e
to
fb9fcc0
Compare
node-tests/addon-tests/addon-test.js
Outdated
.then(response => { | ||
expect(response.statusCode).to.equal(200); | ||
let parsed = esprima.parseScript(response.body); | ||
let bodyStatements = parsed.body |
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 wonder if we could get away with something like this to make it less scary to maintain/add more tests like this in the future?
let parsed = esprima.parseScript(response.body);
let expected = esprima.parseScript(`
export function add(a, b) {
return a + b;
}
`);
expect(parsed).to.deep.equal(expected);
EDIT: Ah right, we're dealing with post-Babel output as well. Might still be worth a try with the corresponding define(...)
?
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.
@dfreeman Good idea! I'll add that in.
@dfreeman I refactored the test as per your suggestion. How does that look? Also, Appveyor seems to be complaining that |
@csantero Looks good! Re: |
Hey, sorry this sat for so long unattended, @csantero. AppVeyor does have yarn available in its test environment, so that shouldn't be the issue. Let's get this updated with latest from |
b3279a3
to
3248de5
Compare
@chriskrycho I don't know what's causing the failure. It works locally |
I'll see if I can poke at it next week if no one beats me to it. Quick request for future reference: please add commits rather than rebasing for PRs to this repo. It's much easier to track what comments were in reference to that way! (No harm done, just figured I'd note it. Should put a note that way in a |
@chriskrycho interesting. I hate it when people update PRs against my repos with merge commits that bring in lots of commits from master. I guess it's personal preference? |
Ha! Yes. I prefer that because it keeps the context. GitHub (and, for that matter, |
Also, my current theory about these failures is that they're just too slow. I don't think this PR is going to be viable until ember-cli-addon-tests sees some major performance improvements. |
We don't currently have an addon acceptance test, but this PR didn't introduce one either :) |
The one test this adds:
ts1
using ember-cli-typescript, with a pre-existing typescript file atapp/add.ts
ts1/add