Skip to content
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

Elevate console warn and error to test failure and component lifecycle clean-up #1748

Merged
merged 6 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
4 changes: 3 additions & 1 deletion angular-workspace/projects/example-client-app/src/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
4 changes: 3 additions & 1 deletion angular-workspace/projects/ni/nimble-angular/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Elevate console warn and error in tests",
"packageName": "@ni/nimble-angular",
"email": "rajsite@users.noreply.github.com",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Make lifecycle callbacks in components call base class consistently",
"packageName": "@ni/nimble-components",
"email": "rajsite@users.noreply.github.com",
"dependentChangeType": "patch"
}
3 changes: 3 additions & 0 deletions packages/nimble-components/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/nimble-components/src/table/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions packages/nimble-components/src/table/models/virtualizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
1 change: 1 addition & 0 deletions packages/nimble-components/src/text-area/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export class TextArea extends FoundationTextArea implements ErrorPattern {
* @internal
*/
public override disconnectedCallback(): void {
super.disconnectedCallback();
this.resizeObserver?.disconnect();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
6 changes: 5 additions & 1 deletion packages/nimble-components/src/utilities/tests/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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$/));
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading