-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Enhance blueprints #4023
Enhance blueprints #4023
Conversation
Can we extract a common shared function from https://github.com/emberjs/data/blob/master/blueprints/adapter/index.js to reuse? |
@rwjblue I extracted the common functionality into Is this something you had in mind? |
Looks perfect, thank you @pangratz! |
The tests fail for the scenarios which check if we extend from the application entity only if it exists. The setup uses the it('adapter extends application adapter, if available', function() {
return generateAndDestroy(['adapter', 'application'], {
afterGenerate: function() {
return generateAndDestroy(['adapter', 'foo'], {
files: [
// ...
]
});
}
});
});
|
May I shamelessly ping @trabus' 👀 here: do you spot any obvious mis-use of the |
I don't have any time to look right now, but you might want to look at https://github.com/trabus/ember-cli-legacy-blueprints/blob/blueprints/node-tests/blueprints/adapter-test.js for tips. Also, I have a PR that needs to be merged into ember-cli for the blueprint-test-helpers to work properly, so you might be running into an issue there. |
@pangratz Actually, it looks like you're missing this: https://github.com/trabus/ember-cli-legacy-blueprints/blob/blueprints/node-tests/blueprints/adapter-test.js#L131 |
@pangratz I made a note to document it. Sorry about that. |
@trabus thank you so much for your help! I will try to help you out with the issue you opened. |
Awesome, looks like it unblocked it! Generating after generating a project is kind of tricky, I wish there were a cleaner way to set that up. |
AppVeyor with its New Year's resolution. Now 78% more green. |
So I just tested this locally and the detection of an existing application adapter / serializer works. I am not sure why the tests are failing... |
So the error here: https://travis-ci.org/emberjs/data/builds/100452185#L7477 We need to cut another release of I will have to take care of this later tonight when I get home to my personal machine. |
@pangratz Unfortunately I've been heavily distracted by life stuff. I will try to cut out some time this week, but I'm not exactly sure when I'll be able to get to it. Sorry for the holdup. |
No worries @trabus! |
Ok, thanks to a new release by señor trabus the tests run now without a timeout 🚀 But, unfortunately it seems that there is a new issue: https://travis-ci.org/emberjs/data/builds/103402540#L7449-L7466: 1) Acceptance: generate and destroy serializer blueprints serializer extends application serializer if it exists:
EqualityError: expected: `app/serializers/foo.js`
+ expected - actual
-import DS from 'ember-data';
-
-export default DS.RESTSerializer.extend({
-});
+import ApplicationSerializer from './application';
at node_modules/ember-cli-blueprint-test-helpers/lib/helpers/blueprint-helper.js:154:14
at Array.forEach (native)
at assertGenerated (node_modules/ember-cli-blueprint-test-helpers/lib/helpers/blueprint-helper.js:151:9)
at lib$rsvp$$internal$$tryCatch (node_modules/rsvp/dist/rsvp.js:493:16)
at lib$rsvp$$internal$$invokeCallback (node_modules/rsvp/dist/rsvp.js:505:17)
at lib$rsvp$$internal$$publish (node_modules/rsvp/dist/rsvp.js:476:11)
at lib$rsvp$asap$$flush (node_modules/rsvp/dist/rsvp.js:1198:9) When testing that it is correctly extended from an existing application serializer, it extends from Now comes the strange part: If the The weird thing is that the test for the application adapter works correctly, though the same test is used (only difference is that an adapter is created instead of a serializer)... @trabus I am sorry to ping you again here but do you have a clue what's going on? |
After further digging the |
@pangratz whats currently blocking this pr? Is the |
@bmac if I understand this correctly then it seems that the tests using ember-cli-blueprint-test-helpers use the blueprints defined in If you checkout the branch of this PR, replace the contents of |
@pangratz I've stepped through it and the problem is definitely not the |
@Turbo87 thanks for checking! Much appreciated. |
@pangratz ember-cli/ember-cli-blueprint-test-helpers@7a709c8 seems to fix the issue |
@pangratz we've just released ember-cli-blueprint-test-helpers v0.10.1 including that bugfix. your test suite should pass fine once you update the dependency. sorry for the troubles! |
@Turbo87 thanks for the quick fix! I've updated the Unfortunately there seems to be an issue with the assertions, checking that a $ npm run-script node-tests
> [email protected] node-tests /home/travis/build/emberjs/data
> node node-tests/nodetest-runner.js
Verifing `.only` in tests
No `.only` found
Acceptance: generate and destroy adapter blueprints
✓ adapter (1811ms)
✓ adapter extends application adapter if it exists (748ms)
✓ adapter with --base-class (340ms)
1) adapter throws when --base-class is same as name
adapters cannot extend from themself. To resolve this, remove the `--base-class` option or change to a different base-class.
adapters cannot extend from themself. To resolve this, remove the `--base-class` option or change to a different base-class.
✓ adapter when is named "application" (650ms)
✓ adapter-test (386ms)
✓ adapter-test for mocha (381ms)
Acceptance: generate and destroy model blueprints
✓ model (424ms)
✓ model with attrs (408ms)
✓ model with belongsTo (395ms)
✓ model with hasMany (450ms)
✓ model with belongsTo and hasMany has both imports (445ms)
✓ model-test (440ms)
✓ model-test for mocha (387ms)
Acceptance: generate and destroy serializer blueprints
✓ serializer (390ms)
✓ serializer extends application serializer if it exists (723ms)
✓ serializer with --base-class (424ms)
2) serializer throws when --base-class is same as name
✓ serializer when is named "application" (472ms)
✓ serializer-test (404ms)
✓ serializer-test for mocha (383ms)
Acceptance: generate and destroy transform blueprints
✓ transform (508ms)
✓ transforms-test (311ms)
✓ transform-test for mocha (462ms)
22 passing (12s)
2 failing
1) Acceptance: generate and destroy adapter blueprints adapter throws when --base-class is same as name:
TypeError: expect(...).to.be.rejectedWith is not a function
at assertThrows (node_modules/ember-cli-blueprint-test-helpers/lib/helpers/blueprint-helper.js:257:32)
at generateAndDestroy (node_modules/ember-cli-blueprint-test-helpers/lib/helpers/blueprint-helper.js:81:12)
at Context.<anonymous> (node-tests/blueprints/adapter-test.js:74:12)
2) Acceptance: generate and destroy serializer blueprints serializer throws when --base-class is same as name:
TypeError: expect(...).to.be.rejectedWith is not a function
at assertThrows (node_modules/ember-cli-blueprint-test-helpers/lib/helpers/blueprint-helper.js:257:32)
at generateAndDestroy (node_modules/ember-cli-blueprint-test-helpers/lib/helpers/blueprint-helper.js:81:12)
at Context.<anonymous> (node-tests/blueprints/serializer-test.js:74:12) |
@pangratz The weird thing is, we have the exact same test in Did you re-run the |
@trabus I ran |
Can you check if |
Hmm, looks like it is installed. $ npm ls chai-as-promised
[email protected]
├─┬ [email protected]
│ └── [email protected]
└─┬ [email protected]
└── [email protected] |
In that case I would suspect that |
@pangratz you can fix the current error by changing |
@Turbo87 you are a 🚀 Thank you kindly! |
@pangratz unfortunately that only fixes the broken |
@pangratz ember-cli/ember-cli-blueprint-test-helpers#47 is solving your |
This change updates the blueprints for adapter and serializer, so they extend from the application entity if it exists: If there is an application adapter - located in `app/adapters/application.js` - and the command `ember g adapter foo` is invoked, the created adapter extends from the application adapter: ```js // app/adapters/foo.js import ApplicationAdapter from './application'; export default ApplicationAdapter.extend(); ``` If there is no application entity, the JSONAPI equivalent is used. Consider no application serializer in `app/serializers/appliation.js` and the command `ember generate serializer foo` is invoked, then the created serializer extends from JSONAPISerializer: ```js // app/serializers/foo.js import JSONAPISerializer from 'ember-data/serializers/json-api'; export default JSONAPISerializer.extend(); ```
This is a followup PR, made due to a comment by @igorT: currently a serializer generate via
ember generate ember-data:serializer foo
extendsDS.JSONAPIAdapter
, where it should extend theApplicationAdapter
. This behavior is already the case when an adapter is generated, so this PR aligns the behavior of the serializer blueprint to the one of the adapter blueprint.TODO
ember g adapter foo
when application adapter is presentindex.js