-
Notifications
You must be signed in to change notification settings - Fork 22
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
Runtime annotations #52
Conversation
Very cool! Thanks so much for this. I'm not entirely clear on how this would work for the user, but that will get clearer with more study-ing. I have one question now: Do I understand correctly that if people want to handle custom annotations they'll need to create a |
Yes. They can name it what they want and put what ever logic in it they want. If we want to provide a default implementation we could provide that as a blueprint. |
I think it would make sense to handle all annotations the same way. |
I simplified some things:
I didn't know how to change the ember's ENV.annotations between running feature acceptance tests. Does any of this break mocha consumers? Let me know your thoughts on where this is going, and I will continue to adjust and clean it up. |
lib/test-runner/qunit/acceptance.js
Outdated
// if a runFeature function is provided, use it to determine if should run the feature | ||
if (typeof(runFeature) === 'function' && !runFeature(feature.annotations)) { | ||
console.log('not running feature: ', feature.title); | ||
let runFeature = yaddaAnnotations.runFeature(feature.annotations); |
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.
if runFeature
would return a function and if we'd call that function from the current scope would we be able to access all the stuff in the current scope?
i.e.:
if (annotations.ignore) {
return function() {
skip(`Feature ${feature.title}`, function (assert) {});
}
}
This way you could do anything to the test, not just skip it. I'm not sure if we could teach this to the user though. We definitely should NOT rely on them having to do this stuff:
test('Scenario: ${scenario.title}', function(assert) {
let self = this;
return new Ember.RSVP.Promise(function(resolve) {
yadda.Yadda(library.default(assert), self).yadda(scenario.steps, { ctx: {} }, function next(err, result) {
if (err) {
throw err;
}
resolve(result);
});
});
I don't know how that would work exactly. We will definitely have to change the mocha runner to do this annotations dance too though.
Is this anywhere close to the concept you are thinking? The code to run the tests is still in the control of the addon. With the way it is coded, there would need to be a decision on what params to pass to the functions. We don't have a use case to do anything more than bypass the test. Do you have a use case in mind? |
Thoughts on where this is going? I'll clean it up and add some doc if so. |
Yeah definitely, I'll try to have a look this evening. I think this is a very worth while extension. We'll have to look into mocha support and upgrade path, from the top of my head. I'll look into this asap. 😄 |
I looked at it yesterday and got kind of lost in getting ember try to install I really like what you've done here and I think it's the correct way to handle annotations. Giving sensible defaults and providing extensibility for future use. @simonihmig and @adamjmcgrath and @mschinis If you guys have time and are still using ember-cli-yadda do you want to read through this and see what you think 😄 |
This branch is failing for mocha because of #53 if you rebase it against master I'll take a look |
hey @albertjan, we're currently not using ember-cli-yadda 😊 |
fda5c6f
to
d26f62c
Compare
@adamjmcgrath can you try this out for mocha? |
d26f62c
to
87b45f5
Compare
$ npm install -g [email protected]
$ ember new yaddatest
$ cd yaddatest/
$ ember install ember-cli-mocha
$ ember install StateFarmIns/ember-cli-yadda#runtime-annotations
$ ember g ember-cli-yadda
$ ember g feature make-feature
$ ember t
...
not ok 1 Chrome 63.0 - TestLoader Failures yaddatest/tests/acceptance/make-feature-test: could not be loaded
---
message: >
Could not find module `yaddatest/tests/helpers/yadda-annotations` imported from `yaddatest/tests/acceptance/make-feature-test` I guess you need a mocha equivalent to blueprints/qunit/ember-cli-yadda/files/tests/helpers/yadda-annotations.js |
@adamjmcgrath I added
It seem to be working. Can you please try again? |
Other than that, LGTM |
Tests are failing when attempting to run with ember 3.0. I think this add-on needs updated to 2.x. Should I open an issue and do a PR? |
@eltjo-k Hey man! How are you doing? Can you give these guys write access to this repo? I don't have the time to maintain it at the moment. |
Alright guys I made you both contributors. if you can decide amongst yourselves that it works merge at will 👍 let me know when I should do a npm release. 😄 |
@albertjan @adamjmcgrath I think this is ready to merge. Any objections? |
Go for it.
…________________________________
From: sfbwalder <[email protected]>
Sent: Saturday, February 17, 2018 7:44:18 PM
To: albertjan/ember-cli-yadda
Cc: Albert-Jan Nijburg; Mention
Subject: Re: [albertjan/ember-cli-yadda] WIP: Runtime annotations (#52)
@albertjan<https://github.com/albertjan> @adamjmcgrath<https://github.com/adamjmcgrath> I think this is ready to merge. Any objections?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#52 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AASa38vtRtqpUwJppISB3odvrZoTCLMOks5tVywSgaJpZM4PbYGf>.
|
@albertjan ready for new release, thx |
@sfbwalder do you have a npm account? |
This is a first cut at allowing the use of annotations to specify which tests to run at run-time as was considered in #17. The code needs some clean-up, so consider this an initial proposal that is very open to everyone's input.
The gist is this:
ember t
or using environment variables to specify the annotations to run.Please provide opinions and I will make updates to the code and this pull request as needed.