fix: repair cleanup testbed script and apply changes (#1057)

* fix: repair cleanup-testbed script * chore: add cleanup-testbed to npm scripts * chore: run cleanup-testbed
intershop · Mar 15, 2022 · a4e403c · a4e403c
1 parent 7aee442
commit a4e403c
Show file tree

Hide file tree

Showing 57 changed files with 136 additions and 343 deletions.
diff --git a/.github/workflows/dead-code.yml b/.github/workflows/dead-code.yml
@@ -25,6 +25,4 @@ jobs:
         run: npm ci
 
       - name: Find Unused TestBed declarations
-        run: |
-          npx ts-node scripts/cleanup-testbed
-          npm run check-no-changes
+        run: npx npm-run-all cleanup-testbed check-no-changes
diff --git a/.github/workflows/development.yml b/.github/workflows/development.yml
@@ -124,9 +124,7 @@ jobs:
         run: npx ts-node scripts/find-dead-code
 
       - name: Find Unused TestBed declarations
-        run: |
-          npx ts-node scripts/cleanup-testbed origin/develop
-          npm run check-no-changes
+        run: npx npm-run-all "cleanup-testbed origin/develop" check-no-changes
 
   Schematics:
     needs: [Quality, Jest]

diff --git a/package.json b/package.json
@@ -32,6 +32,7 @@
     "compile": "tsc --project tsconfig.all.json",
     "dead-code": "npx ts-node scripts/find-dead-code.ts",
     "clean-localizations": "node scripts/clean-up-localizations",
+    "cleanup-testbed": "npx ts-node scripts/cleanup-testbed",
     "spell-check": "cspell lint --no-summary --no-progress \"{src,projects,docs}/**/*.{html,ts,scss,md}\"",
     "spell-check-localizations": "cspell lint --no-summary --no-progress \"src/assets/i18n/{de_DE,en_US,fr_FR}.json\"",
     "clean": "git clean -xdf -e \"*environment.local.ts\" -e \"*environment.development.ts\" -e \"*node_modules\"",

diff --git a/scripts/cleanup-testbed.ts b/scripts/cleanup-testbed.ts
@@ -1,7 +1,5 @@
-import { tsquery } from '@phenomnomnominal/tsquery';
 import { execSync, spawnSync } from 'child_process';
 import { LeftHandSideExpression, Node, Project, PropertyAccessExpression, SourceFile } from 'ts-morph';
-import * as ts from 'typescript';
 
 /* eslint-disable no-console */
 
@@ -38,44 +36,44 @@ console.log(`found ${files.length} test file(s)`);
 
 let processedFiles = 0;
 
+const lintFiles = [];
+
 for (const file of files) {
   processedFiles++;
 
+  const hasTestBedModule = file.getText().includes('configureTestingModule');
+
   const copyPath = `${file.getFilePath()}.ut.spec.ts`;
-  let foundSomething = false;
 
   top: while (true) {
     const percent = ((processedFiles / files.length) * 100).toFixed(0);
 
-    if (
-      !tsquery(
-        file.getSourceFile().compilerNode as unknown as ts.Node,
-        'PropertyAccessExpression[expression.text=TestBed][name.text=configureTestingModule]'
-      ).length
-    ) {
+    if (!hasTestBedModule) {
       console.log(`at ${percent}% - ${path(file)}`, 0, `test(s)`);
     } else {
       const configs: { tb: number; type: string; index: number }[] = [];
 
       let tb = 0;
-      file.forEachDescendant(node => {
-        if (Node.isCallExpression(node) && isTestBedConfigure(node.getExpression())) {
-          const configObject = node.getArguments().find(Node.isObjectLiteralExpression);
-          if (configObject) {
-            for (const type of ['declarations', 'providers', 'imports']) {
-              const array = configObject.getProperty(type);
-              if (Node.isPropertyAssignment(array)) {
-                const initializer = array.getInitializer();
-                if (Node.isArrayLiteralExpression(initializer)) {
-                  for (let index = 0; index < initializer.getElements().length; index++) {
-                    configs.push({ tb, type, index });
+      project.forgetNodesCreatedInBlock(() => {
+        file.forEachDescendant(node => {
+          if (Node.isCallExpression(node) && isTestBedConfigure(node.getExpression())) {
+            const configObject = node.getArguments().find(Node.isObjectLiteralExpression);
+            if (configObject) {
+              for (const type of ['declarations', 'providers', 'imports']) {
+                const array = configObject.getProperty(type);
+                if (Node.isPropertyAssignment(array)) {
+                  const initializer = array.getInitializer();
+                  if (Node.isArrayLiteralExpression(initializer)) {
+                    for (let index = initializer.getElements().length - 1; index >= 0; index--) {
+                      configs.push({ tb, type, index });
+                    }
                   }
                 }
               }
             }
+            tb++;
           }
-          tb++;
-        }
+        });
       });
 
       console.log(`at ${percent}% - ${path(file)}`, configs.length, `test(s)`);
@@ -94,7 +92,7 @@ for (const file of files) {
                 if (Node.isPropertyAssignment(array)) {
                   const initializer = array.getInitializer();
                   if (Node.isArrayLiteralExpression(initializer)) {
-                    const element = initializer.getElements()[config.index].getText();
+                    const element = initializer.getElements()[config.index]?.getText();
 
                     if (initializer.getElements().length === 1) {
                       array.remove();
@@ -113,7 +111,10 @@ for (const file of files) {
                         `${path(file)} - removing '${element}' from ${config.type} in TestBed #${config.tb + 1}`
                       );
                       copy.copyImmediatelySync(file.getFilePath(), { overwrite: true });
-                      foundSomething = true;
+                      lintFiles.push(file.getFilePath());
+                      copy.forget();
+                      copy.deleteImmediatelySync();
+
                       continue top;
                     } catch (err) {
                       continue next;
@@ -130,18 +131,12 @@ for (const file of files) {
   }
 
   if (project.getSourceFile(copyPath)) {
-    project.getSourceFile(copyPath).delete();
-  }
-
-  if (foundSomething) {
-    const filePath = file.getFilePath();
-    execSync(`node scripts/fix-imports ${filePath}`);
-    try {
-      execSync(`npx prettier --write ${filePath}`);
-    } catch (err) {
-      // do nothing
-    }
+    project.getSourceFile(copyPath).deleteImmediatelySync();
   }
 }
 
 project.saveSync();
+
+if (lintFiles.length) {
+  execSync(`npx eslint --fix ${lintFiles.join(' ')}`);
+}
diff --git a/src/app/core/guards/auth.guard.spec.ts b/src/app/core/guards/auth.guard.spec.ts
@@ -1,4 +1,3 @@
-import { Component } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
 import { ActivatedRouteSnapshot, RouterStateSnapshot, UrlTree } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -19,16 +18,8 @@ describe('Auth Guard', () => {
     let store$: Store;
 
     beforeEach(async () => {
-      @Component({ template: 'dummy' })
-      class DummyComponent {}
-
       await TestBed.configureTestingModule({
-        imports: [
-          CoreStoreModule.forTesting(),
-          CustomerStoreModule.forTesting('user'),
-          RouterTestingModule.withRoutes([{ path: 'login', component: DummyComponent }]),
-        ],
-        declarations: [DummyComponent],
+        imports: [CoreStoreModule.forTesting(), CustomerStoreModule.forTesting('user'), RouterTestingModule],
         providers: [{ provide: CookiesService, useFactory: () => instance(mock(CookiesService)) }],
       }).compileComponents();
     });

diff --git a/src/app/core/guards/authorization-toggle.guard.spec.ts b/src/app/core/guards/authorization-toggle.guard.spec.ts
@@ -1,4 +1,3 @@
-import { Component } from '@angular/core';
 import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { Router } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -11,27 +10,23 @@ describe('Authorization Toggle Guard', () => {
   let router: Router;
 
   beforeEach(() => {
-    @Component({ template: 'dummy' })
-    class DummyComponent {}
-
     TestBed.configureTestingModule({
-      declarations: [DummyComponent],
       imports: [
         AuthorizationToggleModule.forTesting('DO_THIS'),
         RouterTestingModule.withRoutes([
           {
             path: 'error',
-            component: DummyComponent,
+            children: [],
           },
           {
             path: 'page1',
-            component: DummyComponent,
+            children: [],
             canActivate: [AuthorizationToggleGuard],
             data: { permission: 'DO_THIS' },
           },
           {
             path: 'page2',
-            component: DummyComponent,
+            children: [],
             canActivate: [AuthorizationToggleGuard],
             data: { permission: 'DO_THAT' },
           },

diff --git a/src/app/core/guards/feature-toggle.guard.spec.ts b/src/app/core/guards/feature-toggle.guard.spec.ts
@@ -1,4 +1,3 @@
-import { ChangeDetectionStrategy, Component } from '@angular/core';
 import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { Router } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -9,27 +8,23 @@ describe('Feature Toggle Guard', () => {
   let router: Router;
 
   beforeEach(() => {
-    @Component({ template: 'dummy', changeDetection: ChangeDetectionStrategy.OnPush })
-    class DummyComponent {}
-
     TestBed.configureTestingModule({
-      declarations: [DummyComponent],
       imports: [
         FeatureToggleModule.forTesting('feature1'),
         RouterTestingModule.withRoutes([
           {
             path: 'error',
-            component: DummyComponent,
+            children: [],
           },
           {
             path: 'feature1',
-            component: DummyComponent,
+            children: [],
             canActivate: [FeatureToggleGuard],
             data: { feature: 'feature1' },
           },
           {
             path: 'feature2',
-            component: DummyComponent,
+            children: [],
             canActivate: [FeatureToggleGuard],
             data: { feature: 'feature2' },
           },

diff --git a/src/app/core/identity-provider/auth0.identity-provider.spec.ts b/src/app/core/identity-provider/auth0.identity-provider.spec.ts
@@ -1,5 +1,4 @@
 import { APP_BASE_HREF } from '@angular/common';
-import { Component } from '@angular/core';
 import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { Router } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -17,9 +16,6 @@ import { ApiTokenService } from 'ish-core/utils/api-token/api-token.service';
 
 import { Auth0Config, Auth0IdentityProvider } from './auth0.identity-provider';
 
-@Component({ template: 'dummy' })
-class DummyComponent {}
-
 const idToken = 'abc123';
 
 const userData: UserData = {
@@ -47,13 +43,12 @@ describe('Auth0 Identity Provider', () => {
 
   beforeEach(() => {
     TestBed.configureTestingModule({
-      declarations: [DummyComponent],
       imports: [
         RouterTestingModule.withRoutes([
-          { path: 'register', component: DummyComponent },
-          { path: 'register/sso', component: DummyComponent },
-          { path: 'account', component: DummyComponent },
-          { path: 'logout', component: DummyComponent },
+          { path: 'register', children: [] },
+          { path: 'register/sso', children: [] },
+          { path: 'account', children: [] },
+          { path: 'logout', children: [] },
         ]),
       ],
       providers: [

diff --git a/src/app/core/routing/category/category.route.spec.ts b/src/app/core/routing/category/category.route.spec.ts
@@ -1,4 +1,3 @@
-import { Component } from '@angular/core';
 import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { Router, UrlMatchResult, UrlSegment } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -166,15 +165,8 @@ describe('Category Route', () => {
   let store$: Store;
 
   beforeEach(() => {
-    @Component({ template: 'dummy' })
-    class DummyComponent {}
-
     TestBed.configureTestingModule({
-      declarations: [DummyComponent],
-      imports: [
-        CoreStoreModule.forTesting(['router']),
-        RouterTestingModule.withRoutes([{ path: '**', component: DummyComponent }]),
-      ],
+      imports: [CoreStoreModule.forTesting(['router']), RouterTestingModule.withRoutes([{ path: '**', children: [] }])],
     });
 
     router = TestBed.inject(Router);

diff --git a/src/app/core/routing/product/product.route.spec.ts b/src/app/core/routing/product/product.route.spec.ts
@@ -1,4 +1,3 @@
-import { Component } from '@angular/core';
 import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { Router, UrlMatchResult, UrlSegment } from '@angular/router';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -338,15 +337,8 @@ describe('Product Route', () => {
   let store$: Store;
 
   beforeEach(() => {
-    @Component({ template: 'dummy' })
-    class DummyComponent {}
-
     TestBed.configureTestingModule({
-      declarations: [DummyComponent],
-      imports: [
-        CoreStoreModule.forTesting(['router']),
-        RouterTestingModule.withRoutes([{ path: '**', component: DummyComponent }]),
-      ],
+      imports: [CoreStoreModule.forTesting(['router']), RouterTestingModule.withRoutes([{ path: '**', children: [] }])],
     });
 
     router = TestBed.inject(Router);

diff --git a/src/app/core/services/api/api.service.spec.ts b/src/app/core/services/api/api.service.spec.ts
@@ -18,7 +18,6 @@ import {
 import { CoreStoreModule } from 'ish-core/store/core/core-store.module';
 import { serverError } from 'ish-core/store/core/error';
 import { loadServerConfigSuccess } from 'ish-core/store/core/server-config';
-import { CustomerStoreModule } from 'ish-core/store/customer/customer-store.module';
 import { getPGID } from 'ish-core/store/customer/user';
 
 import { ApiService, unpackEnvelope } from './api.service';
@@ -554,11 +553,7 @@ describe('Api Service', () => {
     beforeEach(() => {
       TestBed.configureTestingModule({
         // https://angular.io/guide/http#testing-http-requests
-        imports: [
-          CoreStoreModule.forTesting(['configuration', 'serverConfig']),
-          CustomerStoreModule.forTesting('user'),
-          HttpClientTestingModule,
-        ],
+        imports: [CoreStoreModule.forTesting(['configuration', 'serverConfig']), HttpClientTestingModule],
       });
 
       apiService = TestBed.inject(ApiService);

diff --git a/src/app/core/store/customer/customer-store.spec.ts b/src/app/core/store/customer/customer-store.spec.ts
@@ -1,4 +1,3 @@
-import { Component } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
 import { RouterTestingModule } from '@angular/router/testing';
 import { TranslateModule } from '@ngx-translate/core';
@@ -106,9 +105,6 @@ describe('Customer Store', () => {
   } as Promotion;
 
   beforeEach(() => {
-    @Component({ template: 'dummy' })
-    class DummyComponent {}
-
     const categoriesServiceMock = mock(CategoriesService);
     when(categoriesServiceMock.getTopLevelCategories(anyNumber())).thenReturn(of(categoryTree()));
 
@@ -154,18 +150,17 @@ describe('Customer Store', () => {
     when(productPriceServiceMock.getProductPrices(anything())).thenReturn(of([]));
 
     TestBed.configureTestingModule({
-      declarations: [DummyComponent],
       imports: [
         CoreStoreModule.forTesting(['configuration', 'serverConfig'], true),
         CustomerStoreModule,
         RouterTestingModule.withRoutes([
           {
             path: 'account',
-            component: DummyComponent,
+            children: [],
           },
           {
             path: 'checkout/address',
-            component: DummyComponent,
+            children: [],
           },
         ]),
         ShoppingStoreModule,