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

feat(common): Remove uuid and update unique id generation #1408

Merged
1 change: 0 additions & 1 deletion .yarnrc
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
workspaces-experimental true
--install.frozen-lockfile true
6 changes: 3 additions & 3 deletions cypress/integration/Combobox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('Combobox', () => {
});

it('should not show menu', () => {
cy.findByRole('listbox').should('not.be.visible');
cy.findByRole('listbox').should('not.exist');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these were due to a breaking change in Cypress v6.

This "fix" did find legitimate issues with our tests, but I feel like we've lost some semantics. I want to know if the element is no longer accessible to the user. Whether it is still in the DOM or just hidden is an implementation detail. Perhaps I'll make an be.inaccessible assertion that does this.

});

it('should show the clear button', () => {
Expand Down Expand Up @@ -154,7 +154,7 @@ describe('Combobox', () => {
});

it('should close the listbox', () => {
cy.findByRole('listbox').should('not.be.visible');
cy.findByRole('listbox').should('not.exist');
});

context('when the use hits the "a" key', () => {
Expand Down Expand Up @@ -213,7 +213,7 @@ describe('Combobox', () => {
});

it('should close the listbox', () => {
cy.findByRole('listbox').should('not.be.visible');
cy.findByRole('listbox').should('not.exist');
});

it('should keep focus on combobox', () => {
Expand Down
24 changes: 12 additions & 12 deletions cypress/integration/Modal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('Modal', () => {
});

it('should close the modal', () => {
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.exist');
});

it('should transfer focus back to the target button', () => {
Expand All @@ -143,7 +143,7 @@ describe('Modal', () => {
});

it('should close the modal', () => {
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.exist');
});
});

Expand All @@ -153,7 +153,7 @@ describe('Modal', () => {
});

it('should close the modal', () => {
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.exist');
});
});
});
Expand Down Expand Up @@ -189,11 +189,11 @@ describe('Modal', () => {
});

it(`should close the 'Cancel' tooltip`, () => {
cy.findByRole('tooltip', {name: 'Not so sure'}).should('not.be.visible');
cy.findByRole('tooltip', {name: 'Not so sure'}).should('not.exist');
});

it(`should close the modal`, () => {
cy.findByRole('dialog', {name: 'Open Modal'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Open Modal'}).should('not.exist');
});
});
});
Expand Down Expand Up @@ -225,11 +225,11 @@ describe('Modal', () => {
it(`should close the 'OK' tooltip`, () => {
cy.findByRole('tooltip', {
name: 'Really, Really, Really, Really, Really sure',
}).should('not.be.visible');
}).should('not.exist');
});

it(`should close the 'Hidable Popup' popup`, () => {
cy.findByRole('dialog', {name: 'Hidable Popup'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Hidable Popup'}).should('not.exist');
});

it(`should keep the modal open`, () => {
Expand Down Expand Up @@ -268,13 +268,13 @@ describe('Modal', () => {
});

it(`should close the modal`, () => {
cy.findByRole('dialog', {name: 'Non-hidable'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Non-hidable'}).should('not.exist');
});

it(`should close the 'OK' tooltip`, () => {
cy.findByRole('tooltip', {
name: 'Really, Really, Really, Really, Really sure',
}).should('not.be.visible');
}).should('not.exist');
});
});
});
Expand Down Expand Up @@ -518,7 +518,7 @@ describe('Modal', () => {
});

it('should close the modal', () => {
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.exist');
});
});

Expand All @@ -528,7 +528,7 @@ describe('Modal', () => {
});

it('should close the modal', () => {
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Delete Item'}).should('not.exist');
});
});
});
Expand Down Expand Up @@ -633,7 +633,7 @@ describe('Modal', () => {
});

it('should hide the modal', () => {
cy.findByRole('dialog', {name: 'Modal'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Modal'}).should('not.exist');
});

it('should move focus back to the "Open" button', () => {
Expand Down
12 changes: 6 additions & 6 deletions cypress/integration/Popup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('Popup', () => {
});

it('should hide the popup', () => {
cy.findByRole('dialog').should('not.visible');
cy.findByRole('dialog').should('not.exist');
});
});

Expand All @@ -55,7 +55,7 @@ describe('Popup', () => {
});

it('should hide the popup', () => {
cy.findByRole('dialog').should('not.visible');
cy.findByRole('dialog').should('not.exist');
});
});

Expand All @@ -65,7 +65,7 @@ describe('Popup', () => {
});

it('should close the popup', () => {
cy.findByRole('dialog').should('not.visible');
cy.findByRole('dialog').should('not.exist');
});
});
});
Expand Down Expand Up @@ -315,11 +315,11 @@ describe('Popup', () => {
it('should close the tooltip', () => {
cy.findByRole('tooltip', {
name: 'Really long tooltip showing how popup stacks overlap 2',
}).should('not.be.visible');
}).should('not.exist');
});

it('should close the "Really Delete Item" popup', () => {
cy.findByRole('dialog', {name: 'Really Delete Item'}).should('be.not.visible');
cy.findByRole('dialog', {name: 'Really Delete Item'}).should('not.exist');
});

it('should NOT close the "Delete Item" popup', () => {
Expand Down Expand Up @@ -351,7 +351,7 @@ describe('Popup', () => {
});

it('should hide the popup', () => {
cy.findByRole('dialog', {name: 'Popup'}).should('not.be.visible');
cy.findByRole('dialog', {name: 'Popup'}).should('not.exist');
});

it('should move focus back to the "Open" button', () => {
Expand Down
6 changes: 3 additions & 3 deletions cypress/integration/SelectPreview.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('Select', () => {
it('should not be visible', () => {
cy.findByLabelText('Label')
.pipe(h.selectPreview.getMenu)
.should('not.be.visible');
.should('not.exist');
});
});

Expand Down Expand Up @@ -242,7 +242,7 @@ describe('Select', () => {
it('should not be visible', () => {
cy.findByLabelText('Label')
.pipe(h.selectPreview.getMenu)
.should('not.be.visible');
.should('not.exist');
});
});

Expand Down Expand Up @@ -664,7 +664,7 @@ describe('Select', () => {
// it('should not be visible', () => {
// cy.findByLabelText('Label')
// .pipe(h.selectPreview.getMenu)
// .should('not.be.visible');
// .should('not.exist');
// });
// });
// });
Expand Down
22 changes: 11 additions & 11 deletions cypress/integration/Tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('Tooltip', () => {
});

it('should close the tooltip', () => {
cy.findByRole('tooltip').should('not.be.visible');
cy.findByRole('tooltip').should('not.exist');
});
});

Expand All @@ -62,7 +62,7 @@ describe('Tooltip', () => {
});

it('should not close the tooltip', () => {
cy.findByRole('tooltip').should('not.be.visible');
cy.findByRole('tooltip').should('not.exist');
});
});
});
Expand All @@ -82,7 +82,7 @@ describe('Tooltip', () => {
});

it('should close the tooltip', () => {
cy.findByRole('tooltip').should('not.be.visible');
cy.findByRole('tooltip').should('not.exist');
});
});

Expand All @@ -104,7 +104,7 @@ describe('Tooltip', () => {
});

it('should close immediately, not waiting for blur or intent', () => {
cy.findByRole('tooltip', {timeout: 0}).should('not.be.visible');
cy.findByRole('tooltip', {timeout: 0}).should('not.exist');
});
});
});
Expand Down Expand Up @@ -155,14 +155,14 @@ describe('Tooltip', () => {
});

it('the span element should not have an aria-describedby attribute', () => {
cy.get('button').should('not.have.attr', 'aria-describedby');
cy.get('span').should('not.have.attr', 'aria-describedby');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was caught by the change in Cypress v6. Before, not.have.attr would pass even if cy.get('button') never actually matched anything. This was a typo from long ago that Cypress was perfectly happy to incorrectly pass.

});

it('the span element should not have an aria-label attribute', () => {
cy.get('button').should('not.have.attr', 'aria-describedby');
cy.get('span').should('not.have.attr', 'aria-describedby');
});

context('when the "Delete" button is hovered', () => {
context('when the "Some Text" text is hovered', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a copy/paste error. The tooltip isn't from a delete button, but a span with some text.

beforeEach(() => {
cy.get('span').trigger('mouseover');
});
Expand All @@ -176,11 +176,11 @@ describe('Tooltip', () => {
});

it('the span element should not have an aria-describedby attribute', () => {
cy.get('button').should('not.have.attr', 'aria-describedby');
cy.get('span').should('not.have.attr', 'aria-describedby');
});

it('the span element should not have an aria-label attribute', () => {
cy.get('button').should('not.have.attr', 'aria-describedby');
cy.get('span').should('not.have.attr', 'aria-describedby');
});
});
});
Expand All @@ -204,7 +204,7 @@ describe('Tooltip', () => {
});

it('should not show the tooltip', () => {
cy.findByRole('tooltip').should('not.be.visible');
cy.findByRole('tooltip').should('not.exist');
});
});

Expand Down Expand Up @@ -244,7 +244,7 @@ describe('Tooltip', () => {
});

it('should not show the tooltip', () => {
cy.findByRole('tooltip').should('not.be.visible');
cy.findByRole('tooltip').should('not.exist');
});
});

Expand Down
72 changes: 44 additions & 28 deletions cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,33 @@ Cypress.Commands.add('injectAxe', () => {
});

// Needed for https://github.com/Bkucera/cypress-plugin-tab/issues/46
Cypress.Commands.overwrite('visit', (originalFn, url, options = {}) => {
if (typeof url === 'object') {
// eslint-disable-next-line no-param-reassign
url = options.url;
Cypress.Commands.overwrite('visit', (originalFn, urlOrOptions, inputOptions = {}) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file were from Cypress adding type defintions to Cypress.Commands.add and Cypress.Commands.overwrite in Cypress v9. In most cases, this is an improvement. In the case of commands that use Typescript overrides, this causes problems. I've rearranged command overrides and added some any in cases where the Cypress types introduce problems that cannot be resolved.

let options: typeof urlOrOptions;
if (typeof urlOrOptions === 'object') {
options = urlOrOptions;
} else {
options = {url: urlOrOptions, ...inputOptions};
}

return originalFn(url, {
return originalFn({
...options,
onBeforeLoad(win: Window) {
onBeforeLoad(win) {
options.onBeforeLoad?.(win);
supports(); // prime the ally.js supports cache so it doesn't mess with the cypress-plugin-tab
},
});
});

// Add better logging to cy.tab
Cypress.Commands.overwrite('tab', (originalFn, subject, options) => {
const prevSubject = cy.$$(subject || (cy as any).state('window').document.activeElement);
Cypress.Commands.overwrite<'tab', 'element'>('tab', (originalFn, subject, options) => {
// Lots of `any` overrides:

// (cy.$$ as any) - according to the types, `cy.$$` can only take a string which isn't true.
// jQuery can wrap elements directly as well. Source:
// https://github.com/cypress-io/cypress/blob/df5687c65d82e0591256df2dea727e5680baeb82/cli/types/cypress.d.ts#L2285

// (cy as any).state - `cy.state` doesn't have any types since it is considered private.
const prevSubject = (cy.$$ as any)(subject || (cy as any).state('window').document.activeElement);

const log = Cypress.log({
$el: prevSubject,
Expand All @@ -49,16 +58,20 @@ Cypress.Commands.overwrite('tab', (originalFn, subject, options) => {

log.snapshot('before', {next: 'after'});

return Cypress.Promise.try(() => {
return originalFn(subject, options);
})
.then(value => {
log.set('$el', value).snapshot();
return Cypress.$(value);
return (
Cypress.Promise.try(() => {
return originalFn(subject, options);
})
.finally(() => {
log.end();
});
// Cypress types are wrong and think `value` is a `Cypress.Chainable<Subject>` instead of `Subject`
// https://github.com/cypress-io/cypress/pull/19003
.then((value: any) => {
log.set('$el', value).snapshot();
return Cypress.$(value);
})
.finally(() => {
log.end();
}) as any
);
});

declare global {
Expand Down Expand Up @@ -119,15 +132,18 @@ function isKeyOf<T>(obj: T, key: any): key is keyof T {
return typeof key === 'string' && key in obj;
}

Cypress.Commands.overwrite('should', (originalFn, subject, expectation, ...args) => {
const customMatchers = {
'have.ariaDescription': haveAriaDescription(args[0]),
'have.ariaLabel': haveAriaLabel(args[0]),
};
// See if the expectation is a string and if it is a member of Jest's expect
if (isKeyOf(customMatchers, expectation)) {
return originalFn(subject, customMatchers[expectation]);
}
Cypress.Commands.overwrite<'should', 'element'>(
'should',
(originalFn, subject, expectation, ...args) => {
const customMatchers = {
'have.ariaDescription': haveAriaDescription(args[0]),
'have.ariaLabel': haveAriaLabel(args[0]),
};
// See if the expectation is a string and if it is a member of Jest's expect
if (isKeyOf(customMatchers, expectation)) {
return (originalFn as any)(subject, customMatchers[expectation]);
}

return originalFn(subject, expectation, ...args);
});
return originalFn(subject, expectation, ...args);
}
);
Loading