fix(schematics): migrate spec to skipTest to be in line with Angular CLI #2253

Jefiozie · 2019-11-13T07:35:34Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/ngrx/platform/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Documentation has been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

You need to use the spec option for generating test files.

Closes #2242

What is the new behavior?

We are now inline with the Angular CLI skipTest option.

Does this PR introduce a breaking change?

[X] Yes
[ ] No

Other information

Jefiozie · 2019-11-13T07:35:51Z

cc: @timdeschryver

ngrxbot · 2019-11-13T07:42:44Z

Preview docs changes for 866ecc2 at https://previews.ngrx.io/pr2253-866ecc2/

timdeschryver

I found another usage of spec at schematics-core/testing/create-workspace

const defaultModuleOptions = {
  name: 'foo',
  spec: true,
  module: undefined,
  flat: false,
};

I think this const isn't used, so you can remove the whole declaration. After that you'll have to run npm run copy:schematics.

Could you also update the docs please? If you search on --spec in the ngrx.io folder, you will find all occurrences.

Jefiozie · 2019-11-13T15:38:38Z

Thanks for looking into it I will update the PR soon with your findings.

Jefiozie · 2019-11-13T19:55:25Z

So I changed as I believe the correct things now and made updates to the docs. Hope that these are the correct changes.

Jefiozie · 2019-11-13T20:14:10Z

Okay, I don't exactly know why the bazel tests are failing. Is it because of my changes?

timdeschryver · 2019-11-14T08:44:56Z

modules/schematics/src/feature/index.ts

@@ -16,7 +16,7 @@ export default function(options: FeatureOptions): Rule {
        name: options.name,
        path: options.path,
        project: options.project,
-        spec: false,
+        skipTest: true,


Not for now, but could we introduce a skipTest flag for the feature schematic @brandonroberts ?
By default, we create spec files for every schematic, but not for this schematic.

timdeschryver · 2019-11-14T08:48:47Z

@Jefiozie I don't think it's because of the changes, let me re-trigger the CI

timdeschryver

Could you revert the changes made in docs/* please, these are the docs for v4-6 - and are working with previous versions of Angular. Thus, here it will remain spec.

Jefiozie · 2019-11-14T14:09:02Z

Reverted the docs changes, I think we are good now for this PR 👍

timdeschryver · 2019-11-18T18:24:34Z

@Jefiozie could you also add a breaking change note to the PR please, you can find a template in CONTRIBUTING.MD

Jefiozie · 2019-11-18T18:58:32Z

I added the breaking changes request.

brandonroberts · 2020-01-07T13:41:57Z

@Jefiozie will you resolve the merge conflicts? Thanks!

Jefiozie · 2020-01-09T10:38:37Z

I did the merge on my tablet I see that there are problems. Will fix this later this week.

Jefiozie · 2020-01-10T14:24:05Z

@brandonroberts, I merged all the latest changes into my branch. At first I thought the failing tests where because of my changes however I believe that it is because of recent changes in the master branch around #2301.

Could you o maybe @alex-okrushko verify this? (would love to know this for sure 😄 )

timdeschryver · 2020-01-10T22:10:45Z

I also noticed the failing build, should be resolved if #2313 is merged.

Jefiozie · 2020-01-11T08:24:58Z

Thanks @timdeschryver

Jefiozie · 2020-01-11T18:35:59Z

@brandonroberts i think it's good to merge now.

timdeschryver

@Jefiozie can you rebase please (in order to keep the CI happy)?

Closes: ngrx#2242 BREAKING CHANGES: To be inline with the Angular CLI, we migrated the `--spec` to `--skipTest`. By default skipTest is false, this way you will always be provided with `*.spec.ts files` BEFORE: ```sh ng generate action User --spec ``` AFTER: ```sh ng generate action User ```

Closes ngrx#2422

Jefiozie · 2020-01-15T06:08:33Z

@timdeschryver, I believe it should be correct now.

timdeschryver

Thanks @Jefiozie

timdeschryver reviewed Nov 13, 2019

View reviewed changes

timdeschryver added the Needs Cleanup Review changes needed label Nov 13, 2019

Jefiozie requested a review from timdeschryver November 13, 2019 19:57

timdeschryver reviewed Nov 14, 2019

View reviewed changes

timdeschryver approved these changes Nov 14, 2019

View reviewed changes

timdeschryver added Breaking Change and removed Needs Cleanup Review changes needed labels Nov 14, 2019

timdeschryver requested changes Nov 14, 2019

View reviewed changes

timdeschryver added the Needs Cleanup Review changes needed label Nov 14, 2019

Jefiozie force-pushed the PR-2242 branch from 8017e7b to b237cfb Compare November 14, 2019 14:08

timdeschryver approved these changes Nov 14, 2019

View reviewed changes

timdeschryver added 9.x and removed Needs Cleanup Review changes needed labels Nov 14, 2019

Jefiozie force-pushed the PR-2242 branch from b237cfb to 916565e Compare November 18, 2019 18:57

brandonroberts added the Needs Cleanup Review changes needed label Jan 7, 2020

Jefiozie force-pushed the PR-2242 branch from 4845c6e to 4e6dce0 Compare January 10, 2020 14:19

timdeschryver reviewed Jan 14, 2020

View reviewed changes

Jefiozie added 4 commits January 15, 2020 07:01

docs(schematics): migrate spec to skipTest

997dfd9

Closes ngrx#2422

refactor(schematics): migrate spec to skipTest

fb60abf

Closes ngrx#2422

docs(schematics): revert docs changes

866ecc2

Jefiozie force-pushed the PR-2242 branch from 4e6dce0 to 866ecc2 Compare January 15, 2020 06:02

timdeschryver approved these changes Jan 15, 2020

View reviewed changes

brandonroberts merged commit 714ae5f into ngrx:master Jan 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(schematics): migrate spec to skipTest to be in line with Angular CLI #2253

fix(schematics): migrate spec to skipTest to be in line with Angular CLI #2253

Jefiozie commented Nov 13, 2019

Jefiozie commented Nov 13, 2019

ngrxbot commented Nov 13, 2019 •

edited

Loading

timdeschryver left a comment

Jefiozie commented Nov 13, 2019

Jefiozie commented Nov 13, 2019

Jefiozie commented Nov 13, 2019

timdeschryver Nov 14, 2019

timdeschryver commented Nov 14, 2019

timdeschryver left a comment

Jefiozie commented Nov 14, 2019

timdeschryver commented Nov 18, 2019

Jefiozie commented Nov 18, 2019

brandonroberts commented Jan 7, 2020

Jefiozie commented Jan 9, 2020

Jefiozie commented Jan 10, 2020

timdeschryver commented Jan 10, 2020

Jefiozie commented Jan 11, 2020

Jefiozie commented Jan 11, 2020

timdeschryver left a comment

Jefiozie commented Jan 15, 2020

timdeschryver left a comment

fix(schematics): migrate spec to skipTest to be in line with Angular CLI #2253

fix(schematics): migrate spec to skipTest to be in line with Angular CLI #2253

Conversation

Jefiozie commented Nov 13, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Jefiozie commented Nov 13, 2019

ngrxbot commented Nov 13, 2019 • edited Loading

timdeschryver left a comment

Choose a reason for hiding this comment

Jefiozie commented Nov 13, 2019

Jefiozie commented Nov 13, 2019

Jefiozie commented Nov 13, 2019

timdeschryver Nov 14, 2019

Choose a reason for hiding this comment

timdeschryver commented Nov 14, 2019

timdeschryver left a comment

Choose a reason for hiding this comment

Jefiozie commented Nov 14, 2019

timdeschryver commented Nov 18, 2019

Jefiozie commented Nov 18, 2019

brandonroberts commented Jan 7, 2020

Jefiozie commented Jan 9, 2020

Jefiozie commented Jan 10, 2020

timdeschryver commented Jan 10, 2020

Jefiozie commented Jan 11, 2020

Jefiozie commented Jan 11, 2020

timdeschryver left a comment

Choose a reason for hiding this comment

Jefiozie commented Jan 15, 2020

timdeschryver left a comment

Choose a reason for hiding this comment

ngrxbot commented Nov 13, 2019 •

edited

Loading