Elevate console warn and error to test failure and component lifecycl…

…e clean-up (#1748)

# Pull Request

## 🤨 Rationale

Fixes a couple of small issues noticed recently:

- Tests were printing out console warnings
- Some component callback lifecycles were not calling their super
implementation
- When test failures happened the current spec would stop making it
difficult to see all the failures and handle them

## 👩‍💻 Implementation

- Elevated console warn and console errors to test failures
- Updated lifecycle callbacks to call super (and renamed some methods
that weren't actually lifecycle callbacks, which was my idea, I was
wrong 😅). Created a Manual Regression Validation issue to hopefully
track that a little better in the future:
#1747
- Updated the jasmine configuration to continue running a test suite
even on failure. We can see if it is useful and easily switch back if it
is annoying. Was useful locally.
- Filed an issue for some Wafer Map tests that are running incorrectly:
#1746

## 🧪 Testing

Rely on CI.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

angular-workspace/projects/example-client-app/karma.conf.js

@@ -20,6 +20,7 @@ module.exports = function (config) {
         // the possible options are listed at https://jasmine.github.io/api/edge/Configuration.html
         // for example, you can disable the random execution with `random: false`
         // or set a specific seed with `seed: 4321`
+        stopSpecOnExpectationFailure: false
       },
       clearContext: false // leave Jasmine Spec Runner output visible in browser
     },

angular-workspace/projects/example-client-app/src/test.ts

@@ -18,6 +18,8 @@ getTestBed().initTestEnvironment(
     }
 );
 
-// Elevate console errors to test failures
+// Elevate console errors and warnings to test failures
 // eslint-disable-next-line no-console, @typescript-eslint/no-explicit-any
 console.error = (data: any): void => fail(data);
+// eslint-disable-next-line no-console, @typescript-eslint/no-explicit-any
+console.warn = (data: any): void => fail(data);

angular-workspace/projects/ni/nimble-angular/karma.conf.js

@@ -26,6 +26,7 @@ module.exports = config => {
                 // the possible options are listed at https://jasmine.github.io/api/edge/Configuration.html
                 // for example, you can disable the random execution with `random: false`
                 // or set a specific seed with `seed: 4321`
+                stopSpecOnExpectationFailure: false
             },
             clearContext: false // leave Jasmine Spec Runner output visible in browser
         },

angular-workspace/projects/ni/nimble-angular/src/thirdparty/directives/tests/value_accessor_integration.spec.ts

@@ -183,6 +183,8 @@ class TestNgSelectOption extends NgSelectOption {}
       input.nativeElement.value = 'updatedValue';
 
       dispatchEvent(input.nativeElement, 'change');
+      // [Nimble] Add a known empty expect to suppress warning
+      expect().nothing();
     });
 
     it('should support <textarea>', () => {
@@ -276,6 +278,8 @@ class TestNgSelectOption extends NgSelectOption {}
 
         input.nativeElement.value = '5';
         dispatchEvent(input.nativeElement, 'change');
+        // [Nimble] Add a known empty expect to suppress warning
+        expect().nothing();
       });
 
       it('when value is cleared programmatically', () => {

angular-workspace/projects/ni/nimble-angular/test.ts

@@ -18,6 +18,8 @@ getTestBed().initTestEnvironment(
     }
 );
 
-// Elevate console errors to test failures
+// Elevate console errors and warnings to test failures
 // eslint-disable-next-line no-console, @typescript-eslint/no-explicit-any
 console.error = (data: any): void => fail(data);
+// eslint-disable-next-line no-console, @typescript-eslint/no-explicit-any
+console.warn = (data: any): void => fail(data);

change/@ni-nimble-angular-b7425a2f-a846-408c-a2e4-bdc0b63594cf.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Elevate console warn and error in tests",
+  "packageName": "@ni/nimble-angular",
+  "email": "[email protected]",
+  "dependentChangeType": "none"
+}

change/@ni-nimble-components-fc5a5bd1-618c-4519-817f-fcbab48f03bb.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Make lifecycle callbacks in components call base class consistently",
+  "packageName": "@ni/nimble-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/nimble-components/karma.conf.js

@@ -142,6 +142,9 @@ module.exports = config => {
             }
         },
         client: {
+            jasmine: {
+                stopSpecOnExpectationFailure: false
+            },
             captureConsole: true
         },
         logLevel: config.LOG_ERROR // to disable the WARN 404 for image requests

packages/nimble-components/src/label-provider/base/index.ts

@@ -26,6 +26,7 @@ export abstract class LabelProviderBase<
     }
 
     public override disconnectedCallback(): void {
+        super.disconnectedCallback();
         this.propertyNotifier.unsubscribe(this);
         if (this.themeProvider) {
             for (const token of Object.values(this.supportedLabels)) {

packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts

@@ -1788,7 +1788,7 @@ describe('RichTextEditor', () => {
 
         expect(inputEventListener.spy).toHaveBeenCalledTimes(1);
 
-        await pageObject.setEditorTextContent('');
+        await pageObject.replaceEditorContent('');
         await inputEventListener.promise;
 
         expect(inputEventListener.spy).toHaveBeenCalledTimes(1);

packages/nimble-components/src/table/index.ts

@@ -326,7 +326,7 @@ export class Table<
     public override connectedCallback(): void {
         super.connectedCallback();
         this.initialize();
-        this.virtualizer.connectedCallback();
+        this.virtualizer.connect();
         this.viewport.addEventListener('scroll', this.onViewPortScroll, {
             passive: true
         });
@@ -336,7 +336,7 @@ export class Table<
 
     public override disconnectedCallback(): void {
         super.disconnectedCallback();
-        this.virtualizer.disconnectedCallback();
+        this.virtualizer.disconnect();
         this.viewport.removeEventListener('scroll', this.onViewPortScroll);
         document.removeEventListener('keydown', this.onKeyDown);
         document.removeEventListener('keyup', this.onKeyUp);

packages/nimble-components/src/table/models/virtualizer.ts

@@ -53,12 +53,12 @@ export class Virtualizer<TData extends TableRecord = TableRecord> {
         });
     }
 
-    public connectedCallback(): void {
+    public connect(): void {
         this.viewportResizeObserver.observe(this.table.viewport);
         this.updateVirtualizer();
     }
 
-    public disconnectedCallback(): void {
+    public disconnect(): void {
         this.viewportResizeObserver.disconnect();
     }
 

packages/nimble-components/src/text-area/index.ts

@@ -71,6 +71,7 @@ export class TextArea extends FoundationTextArea implements ErrorPattern {
      * @internal
      */
     public override disconnectedCallback(): void {
+        super.disconnectedCallback();
         this.resizeObserver?.disconnect();
     }
 

packages/nimble-components/src/utilities/tests/setup-configuration.ts

@@ -0,0 +1,7 @@
+// Scripts that should run at the very beginning of jasmine tests
+
+// Elevate console errors and warnings to test failures
+// eslint-disable-next-line no-console, @typescript-eslint/no-explicit-any
+console.error = (data: any): void => fail(data);
+// eslint-disable-next-line no-console, @typescript-eslint/no-explicit-any
+console.warn = (data: any): void => fail(data);

packages/nimble-components/src/utilities/tests/setup.ts

@@ -4,5 +4,9 @@ function importAll(r: __WebpackModuleApi.RequireContext): void {
     r.keys().forEach(r);
 }
 
-// Explicitly add to browser test
+// browser test configuration setup
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+require('./setup-configuration.js');
+
+// all browser test scripts
 importAll(require.context('../../', true, /\.spec\.js$/));

packages/nimble-components/src/wafer-map/tests/prerendering.coloring.spec.ts

@@ -243,7 +243,10 @@ describe('Wafermap Prerendering module', () => {
         });
     });
 
-    describe('with non numeric values', () => {
+    // Test disabled as it currently does not have expect statements
+    // that run to perform any useful validation
+    // See: https://github.com/ni/nimble/issues/1746
+    xdescribe('with non numeric values', () => {
         const dieDimensions = { width: 10, height: 10 };
         const dieLabelsSuffix = '';
         const dieLabelsHidden = true;
@@ -287,7 +290,10 @@ describe('Wafermap Prerendering module', () => {
         });
     });
 
-    describe('with undefined values', () => {
+    // Test disabled as it currently does not have expect statements
+    // that run to perform any useful validation
+    // See: https://github.com/ni/nimble/issues/1746
+    xdescribe('with undefined values', () => {
         const dieDimensions = { width: 10, height: 10 };
         const dieLabelsSuffix = '';
         const dieLabelsHidden = true;