Create cypress command namespacing util #9150
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| chore: | ||
| - Create cypress command namespacing util ([#9150](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9150)) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| /** | ||
| * Initializes a command namespace on the cy object | ||
| * | ||
| * Ex usage: | ||
| * initCommandNamespace(cy, 'osd'); // initializes osd namespace (only needed once per ns) | ||
| * cy.osd.add('myNewCommand', (myArg) => ...); // register command to namespace | ||
| * cy.osd.myNewCommand('someArg'); // executes command | ||
| * | ||
| */ | ||
| export default function initCommandNamespace(cy, namespace) { | ||
| /** | ||
| * this proxy is responsible for intercepting access to properties | ||
| * it's what allows us to dynamically define properties at runtime like cy.osd.myNewCommand | ||
| */ | ||
| cy[namespace] = new Proxy( | ||
|
|
||
| { | ||
| // register a new namespaced command with cypress (ex. osd:myNewCommand) | ||
| add(commandName, callback) { | ||
| Cypress.Commands.add(namespaceCommand(commandName), callback); | ||
| }, | ||
| }, | ||
| { | ||
| get(target, property) { | ||
| if (target[property]) { | ||
| // return reserved property (add method) | ||
| return target[property]; | ||
| } | ||
|
|
||
| // return the mapped namespace command (ex. myNewCommand returns the osd:myNewCommand command) | ||
| return cy[namespaceCommand(property)]; | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| function namespaceCommand(commandName) { | ||
| return `${namespace}:${commandName}`; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ export function removeSampleDataAndWorkspace(url, workspaceName) { | |||||
| describe('removing workspace/sampledata', () => { | ||||||
| it('remove workspace', () => { | ||||||
|
There was a problem hiding this comment. not specific to this pr but how come this a utils file if it's a test ? There was a problem hiding this comment. I believe because this can be re-used across a number of tests. For example, I write a new test and I want to remove a workspace as a part of this test and get solid logs of which parts failed or succeed as a part of that process. |
||||||
| cy.visit(`${url}/app/workspace_list`); | ||||||
| cy.osd.openWorkspaceDashboard(workspaceName); | ||||||
| cy.getElementByTestId('toggleNavButton').eq(0).should('exist').click(); | ||||||
| cy.wait(3000); | ||||||
|
There was a problem hiding this comment. not specifc to this pr but how come we opt for the cy.wait instead of using something like example: https://docs.cypress.io/app/core-concepts/retry-ability#Timeouts so we could put an increased timeout on the next line. cy.getElementByTestId (we might have to modify it if it's not available for that function). like
Suggested change
then instead of an ambiguous wait time which could get flaky it will just keep retrying for the element until it sees it for 10000 ms There was a problem hiding this comment. Yeah, I agree we shouldn't use arbitrary waits given these are flaky and introduce race conditions. I'm a little weary of addressing in this PR since I'm not confident that I'll be able to address easily with the confidence that I won't break anything given I have somewhat limited context in our integration tests. |
||||||
| cy.getElementByTestId('collapsibleNavAppLink-workspace_detail').should('exist').click(); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -182,13 +182,21 @@ declare namespace Cypress { | |
| */ | ||
| drag<S = any>(targetSelector: string): Chainable<S>; | ||
|
|
||
| // osd namespace | ||
| osd: { | ||
| /** | ||
| * Creates workspace and attaches it to the provided data source | ||
| * It also saves the created workspace id as the alias @WORKSPACE_ID | ||
| */ | ||
| createInitialWorkspaceWithDataSource<S = any>( | ||
| dataSourceTitle: string, | ||
|
There was a problem hiding this comment. nit: while we are in the neighborhood i wonder if it makes sense to pass an object incase we want to add more details to the workspace or the data source like the other thing is i'm not super positive if data source title is unique all the time. i think through the UI it is impossible to recreate a duplicate name but via saved objects and APIs you can create a data source with a title identical to another data source. should we not be utilizing the ID? data source should be a saved object and saved object id is always unique. |
||
| workspaceName: string | ||
|
There was a problem hiding this comment. nit: workspace should be a saved object too i believe. if it is then i think workspaceTitle would be more accurate since the saved object the field is title i believe There was a problem hiding this comment. I'm a little hesitant to change the terminology just because it looks like |
||
| ): Chainable<S>; | ||
|
|
||
| /** | ||
| * Opens workspace dashboard | ||
| */ | ||
| openWorkspaceDashboard<S = any>(workspaceName: string): Chainable<S>; | ||
| }; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a comment briefly explaining what this does can be helpful as the function itself is pretty abstract, and if we can have a way of making the Typescript declaration files work with this pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 I'll add comments here to better explain this module. It's challenging to get meaningful TS types for free here since namespaces and their commands are dynamically defined at module resolution, but I can definitely take a look at making the definitions for the two commands I migrated over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments and type definitions. Let me know what you think.