Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions .github/workflows/cypress_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ on:
branches: ['**']
paths-ignore:
- '**/*.md'
- 'docs/**'
- '.lycheeignore'
- 'CODEOWNERS'
- 'changelogs/fragments/**'
workflow_dispatch:
inputs:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/cypress_workflow_with_s3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ on:
branches: ['main', '[0-9]+\.x', '[0-9]+\.[0-9]+']
paths-ignore:
- '**/*.md'
- 'docs/**'
- '.lycheeignore'
- 'CODEOWNERS'
- 'changelogs/fragments/**'
workflow_dispatch:
inputs:
Expand Down
2 changes: 2 additions & 0 deletions changelogs/fragments/9483.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test:
- Update caniuse and return promise and properly chain the async operations ([#9483](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9483))
6 changes: 3 additions & 3 deletions cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ module.exports = defineConfig({
runMode: 2,
openMode: 0,
},
viewportWidth: 2000,
viewportHeight: 1320,
viewportWidth: 1920,
viewportHeight: 1080,
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

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

Do we have any screenshot comparisons we have to worry about breaking here?

env: {
ENGINE: {
name: 'default',
Expand Down Expand Up @@ -50,7 +50,7 @@ module.exports = defineConfig({
e2e: {
baseUrl: 'http://localhost:5601',
specPattern: 'cypress/integration/**/*.spec.{js,jsx,ts,tsx}',
testIsolation: false,
testIsolation: true,
Copy link
Collaborator

@angle943 angle943 Mar 4, 2025

Choose a reason for hiding this comment

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

previously i was wondering why this was set to false. If tests pass then yeah this should be set to true.

But to give some background knowledge. In other environments we set this to false. This is because some of our tests require authentication and that does not persist if this is set to true

setupNodeEvents,
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,20 @@ const runSavedSearchTests = () => {

it(`should successfully update a saved search for ${config.testName}`, () => {
// using a POST request to create a saved search to load
postRequestSaveSearch(config);
updateSavedSearchAndSaveAndVerify(config, workspaceName, DATASOURCE_NAME, false);
cy.wrap(null).then(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Is this to ensure that the async operations are completed before continuing to the assertions?

return postRequestSaveSearch(config).then(() => {
updateSavedSearchAndSaveAndVerify(config, workspaceName, DATASOURCE_NAME, false);
});
});
});

it(`should successfully save a saved search as a new saved search for ${config.testName}`, () => {
// using a POST request to create a saved search to load
postRequestSaveSearch(config);
updateSavedSearchAndSaveAndVerify(config, workspaceName, DATASOURCE_NAME, true);
cy.wrap(null).then(() => {
return postRequestSaveSearch(config).then(() => {
updateSavedSearchAndSaveAndVerify(config, workspaceName, DATASOURCE_NAME, true);
});
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

why was this change needed? Your description says something about bumping the timeout, but is this section related to that?

});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,14 @@ const runRecentQueryTests = () => {
cy.setDataset(config.dataset, DATASOURCE_NAME, config.datasetType);
cy.setQueryLanguage(config.language);
setDatePickerDatesAndSearchIfRelevant(config.language);

const currentLang = BaseQuery[config.datasetType][config.language];
const currentBaseQuery = currentLang.query;
const currentWhereStatement = currentLang.where;
const baseQueryString = `${currentLang.query} ${config.dataset} ${currentLang.where}`;

TestQueries.forEach((query) => {
cy.setQueryEditor(
currentBaseQuery + config.dataset + currentWhereStatement + query,
{},
true
);
cy.setQueryEditor(`${baseQueryString} ${query}`, {}, true);
});

cy.getElementByTestId('queryEditorFooterToggleRecentQueriesButton').click({
force: true,
});
Expand Down Expand Up @@ -129,15 +127,13 @@ const runRecentQueryTests = () => {
steps.forEach(({ action }, stepIndex) => {
action();
cy.getElementByTestIdLike('row-').each(($row, rowIndex) => {
let expectedQuery = '';
let expectedQuery;
if (rowIndex === 1 && stepIndex >= 2) {
expectedQuery =
currentBaseQuery + config.alternativeDataset + reverseList[rowIndex];
expectedQuery = `${currentLang.query} ${config.alternativeDataset} ${reverseList[rowIndex]}`;
} else if (rowIndex === 0 && stepIndex >= 1) {
expectedQuery = currentBaseQuery + config.dataset + reverseList[rowIndex];
expectedQuery = `${currentLang.query} ${config.dataset} ${reverseList[rowIndex]}`;
} else {
expectedQuery =
currentBaseQuery + config.dataset + currentWhereStatement + reverseList[rowIndex];
expectedQuery = `${baseQueryString} ${reverseList[rowIndex]}`;
}
expect($row.text()).to.contain(expectedQuery);
});
Expand All @@ -148,18 +144,19 @@ const runRecentQueryTests = () => {
cy.setDataset(config.dataset, DATASOURCE_NAME, config.datasetType);
cy.setQueryLanguage(config.language);
setDatePickerDatesAndSearchIfRelevant(config.language);

const currentLang = BaseQuery[config.datasetType][config.language];
const currentBaseQuery = currentLang.query;
const currentWhereStatement = currentLang.where;
const baseQueryString = `${currentLang.query} ${config.dataset} ${currentLang.where}`;

const testQueries = [
currentBaseQuery + config.dataset + currentWhereStatement + ' status_code = 504', // valid
currentBaseQuery + config.dataset + currentWhereStatement, // invalid
`${baseQueryString} status_code = 504`, // valid
baseQueryString, // invalid
];

testQueries.forEach((query, index) => {
cy.setQueryEditor(query, {}, true);
cy.setQueryEditor(query, {}, true);
if (!index)
// it remains expanded for the second iteration, no need to expand it again
cy.getElementByTestId('queryEditorFooterToggleRecentQueriesButton').click({
force: true,
});
Expand All @@ -177,15 +174,17 @@ const runRecentQueryTests = () => {
setDatePickerDatesAndSearchIfRelevant(config.language);
// Precondition: run some queries first
const currentLang = BaseQuery[config.datasetType][config.language];
const currentBaseQuery = currentLang.query + config.dataset + currentLang.where;
const baseQueryString = `${currentLang.query} ${config.dataset} ${currentLang.where}`;

const queries = [...TestQueries].splice(0, 3);
queries.forEach((query) => {
cy.setQueryEditor(currentBaseQuery + query, {}, true);
cy.setQueryEditor(`${baseQueryString} ${query}`, {}, true);
});

cy.getElementByTestId('queryEditorFooterToggleRecentQueriesButton').click({
force: true,
});
const expectedQuery = currentBaseQuery + queries[0];
const expectedQuery = `${baseQueryString} ${queries[0]}`;
cy.getElementByTestIdLike('row-') // check query in original position
.eq(2)
.focus()
Expand Down
8 changes: 4 additions & 4 deletions cypress/utils/apps/query_enhancements/saved.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,11 @@ const getSavedObjectPostBody = (config, workspaceId, datasourceId, indexPatternI
* @param {SavedTestConfig} config - the relevant config for the test case
*/
export const postRequestSaveSearch = (config) => {
cy.get('@WORKSPACE_ID').then((workspaceId) => {
cy.get('@DATASOURCE_ID').then((datasourceId) => {
cy.get('@INDEX_PATTERN_ID').then((indexPatternId) => {
return cy.get('@WORKSPACE_ID').then((workspaceId) => {
return cy.get('@DATASOURCE_ID').then((datasourceId) => {
return cy.get('@INDEX_PATTERN_ID').then((indexPatternId) => {
// POST a saved search
cy.request({
return cy.request({
Copy link
Collaborator

Choose a reason for hiding this comment

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

why returns?

Copy link
Member Author

Choose a reason for hiding this comment

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

i wasn't originally seeing things wait for the saved search to load which should be async so i'm returning the promise chain so that then we can chain off the response and then proceed to validate. but i could be missing a wait from intercepting somehting.

method: 'POST',
url: `/w/${workspaceId}/api/saved_objects/search?overwrite=true`,
headers: {
Expand Down
18 changes: 12 additions & 6 deletions cypress/utils/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@

// --- Typed commands --

Cypress.Commands.add('getElementByTestId', (testId, options = {}) => {
return cy.get(`[data-test-subj="${testId}"]`, options);
});
Cypress.Commands.add(
'getElementByTestId',
(testId, options = { timeout: Cypress.config('defaultCommandTimeout') }) => {
return cy.get(`[data-test-subj="${testId}"]`, options);
}
);

Cypress.Commands.add('getElementByTestIdLike', (testId, options = {}) => {
return cy.get(`[data-test-subj*="${testId}"]`, options);
});
Cypress.Commands.add(
'getElementByTestIdLike',
(testId, options = { timeout: Cypress.config('defaultCommandTimeout') }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

there should be a way to just type the options so that it has all the things that the usual options for cy.get() has. This way if we ever need to add more stuff in the future, we don't need to explictely update the typing

Copy link
Member Author

Choose a reason for hiding this comment

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

i think you already set this value at the cypress config level so i was kind of shooting in the dark with this one and can prolly remove not positive if the option will be passed down nested levels would make sense to me. but basically anyone could override it. unless you are trying to say it would be better to have a set an options that we would let people set and not let them configure it past that

return cy.get(`[data-test-subj*="${testId}"]`, options);
}
);

Cypress.Commands.add('getElementsByTestIds', (testIds, options = {}) => {
const selectors = [testIds].flat(Infinity).map((testId) => `[data-test-subj="${testId}"]`);
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5566,9 +5566,9 @@ camelize@^1.0.0:
integrity sha1-FkpUg+Yw+kMh5a8HAg5TGDGyYJs=

caniuse-lite@^1.0.30001464, caniuse-lite@^1.0.30001640:
version "1.0.30001659"
resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001659.tgz"
integrity sha512-Qxxyfv3RdHAfJcXelgf0hU4DFUVXBGTjqrBUZLUh8AtlGnsDo+CnncYtTd95+ZKfnANUOzxyIQCuU/UeBZBYoA==
version "1.0.30001701"
resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001701.tgz"
integrity sha512-faRs/AW3jA9nTwmJBSO1PQ6L/EOgsB5HMQQq4iCu5zhPgVVgO/pZRHlmatwijZKetFw8/Pr4q6dEN8sJuq8qTw==

caseless@~0.12.0:
version "0.12.0"
Expand Down
Loading