Follow up on #9048 by updating data and utilities #9060

ananzh · 2024-12-16T18:36:26Z

Description

Merge feature branch PRs:
Add workspace utility #9038 Add test data for query enhancement functional tests #9006
Add sample test a_check.spec.js
Update utilities and data

To Run

// Download opensearch artifact and unzip
wget https://artifacts.opensearch.org/releases/bundle/opensearch/2.17.0/opensearch-2.17.0-linux-x64.tar.gz
tar -xvf opensearch-2.17.0-linux-x64.tar.gz
// remove security
./bin/opensearch-plugin remove opensearch-security
./opensearch-tar-install.sh
// enable workspace in confid/opensearch_dashboards.yml
workspace.enabled: true
opensearch.ignoreVersionMismatch: true
// run osd
yarn start:enhancements --no-base-path

Issues Resolved

NA

Changelog

skip

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

Feature branch PRs: opensearch-project#9038 opensearch-project#9006 Signed-off-by: Anan <[email protected]>

codecov · 2024-12-16T18:52:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.86%. Comparing base (442e11e) to head (474c2e5).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9060   +/-   ##
=======================================
  Coverage   60.86%   60.86%           
=======================================
  Files        3808     3808           
  Lines       91209    91209           
  Branches    14410    14410           
=======================================
  Hits        55516    55516           
+ Misses      32156    32153    -3     
- Partials     3537     3540    +3

Flag	Coverage Δ
Linux_1	`29.02% <ø> (ø)`
Linux_2	`56.38% <ø> (ø)`
Linux_3	`37.93% <ø> (ø)`
Linux_4	`29.01% <ø> (ø)`
Windows_1	`29.03% <ø> (ø)`
Windows_2	`56.34% <ø> (ø)`
Windows_3	`37.93% <ø> (+<0.01%)`	⬆️
Windows_4	`29.01% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LDrago27 · 2024-12-16T19:53:00Z

cypress/integration/apps/query_enhancements/constants.js

+
+export const BASE_PATH = Cypress.config().baseUrl;
+export const DATASOURCE_NAME = 'test_cluster';
+export const DATASOURCE_URL = 'http://localhost:9200';


We can update the DataSource_URL to use the openSearchUrl: 'http://localhost:9200' defined within this file

will clean out all the base path and url in another follow-up

LDrago27 · 2024-12-16T23:16:55Z

.github/workflows/cypress_workflow.yml

@@ -90,7 +90,7 @@ jobs:
      options: --user 1001
    env:
      START_CMD: ${{ matrix.config == 'query_enhanced' &&
-        'node scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true --csp.warnLegacyBrowsers=false --uiSettings.overrides["query:enhancements:enabled"]=true --uiSettings.overrides[''home:useNewHomePage'']=true --data_source.enabled=true --opensearch.ignoreVersionMismatch=true' ||
+        'node scripts/opensearch_dashboards --dev --no-base-path --no-watch --savedObjects.maxImportPayloadBytes=10485760 --server.maxPayloadBytes=1759977 --logging.json=false --data.search.aggs.shardDelay.enabled=true --csp.warnLegacyBrowsers=false --uiSettings.overrides["query:enhancements:enabled"]=true --uiSettings.overrides[''home:useNewHomePage'']=true --data_source.enabled=true --workspace.enable=true --opensearch.ignoreVersionMismatch=true' ||


Typo --workspace.enable=true should be --workspace.enabled=true

nice. thanks a lot.

We need to have tests run with and without workspaces being enabled.

kavilla · 2024-12-18T01:29:23Z

...ion/core-opensearch-dashboards/opensearch-dashboards/apps/query_enhancements/a_check.spec.js

+    // Load test data
+    cy.setupTestData(
+      [
+        'cypress/fixtures/query_enhancements/data-logs-1/data_logs_small_time_1.mapping.json',


if we refactor the structure of the fixtures to not care about the plugin for example cypress/fixtures/{file name}

we could make setupTestData just automatically set the path so that all we have to do is pass the file name for example

setupTestData('small_time_range_1.mapping.json')

then in the setupTestData we automatically just preprend cypress/fixtures/ to it. this would make it easier and less prone to error if the file path is too long

kavilla · 2024-12-18T01:30:21Z

cypress/utils/apps/query_enhancements/commands.js

+ * @param {string} [options.credentials.username] Username for basic auth
+ * @param {string} [options.credentials.password] Password for basic auth
+ */
+Cypress.Commands.add('addDataSource', (options) => {


whats specific about query enhancements ? this seems like it should be globally used

kavilla · 2024-12-18T01:32:06Z

cypress/utils/dashboards/workspace-plugin/commands.js

why workspace-plugin ? query_enhancements is a plugin as well so we should just remove the plugin

kavilla · 2024-12-18T01:32:39Z

cypress/support/e2e.js

@@ -5,3 +5,28 @@

 import '../utils/commands';
 import '../utils/apps/commands';
+import '../utils/dashboards/workspace-plugin/commands';


imo the commands should be exported in dashboards/commands and then we just import it instead of nesting a plugin

kavilla · 2024-12-18T01:33:50Z

cypress/scripts/generate_data/index.js

+const { TestDataGenerator } = require('./test_data_generator');
+
+// Need to update different path for multiple clusters
+const DEFAULT_PATH =


nested default path that points to a plugin fixture. we shouldn't do this we should use cypress configuration to set these values in the env this will likely get lost

kavilla · 2024-12-18T01:36:23Z

cypress/scripts/generate_data/index.js

what javascript?

kavilla · 2024-12-18T01:38:19Z

...ion/core-opensearch-dashboards/opensearch-dashboards/apps/query_enhancements/queries.spec.js

why we moving to integration/core-opensearch-dashboards/opensearch-dashboards

this is too long and what other tests would exist in this repo besides the core opensearch dashboards tests?

kavilla · 2024-12-18T01:39:14Z

cypress/lib/test-fixture-handler.js

+ */
+
+export class TestFixtureHandler {
+  constructor(inputTestRunner, openSearchUrl = 'localhost:9200') {


we aren't we pulling from the cypyress environment variable ?

kavilla · 2024-12-18T01:40:49Z

cypress/lib/test-fixture-handler.js

+  }
+
+  _extractIndexNameFromPath(filepath) {
+    const filename = filepath.split('/').pop();


i dont think this is worth it and requires knowing the system. to the point where it might just be easier to require the index name as apart of the file fixture

we don't use this one. it is for easy migration.

kavilla · 2024-12-18T01:42:33Z

cypress/utils/dashboards/commands.js

+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+Cypress.Commands.add('selectFromDataSourceSelector', (dataSourceTitle, dataSourceId) => {


seeing this as a datasource command as well im not positive what goes into utils/commands and what goes into query_enhancements/commands which all have a data source command

we don't use this one. it is for easy migration.

kavilla · 2024-12-18T01:42:55Z

cypress/utils/dashboards/commands.js

+
+Cypress.Commands.add('selectFromDataSourceSelector', (dataSourceTitle, dataSourceId) => {
+  // clean up the input field
+  cy.wait(1000);


:( cy.wait make tests flaky

we don't use this one. it is for easy migration.

kavilla · 2024-12-18T01:45:44Z

cypress/utils/dashboards/remove_workspace.js

+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+export function removeSampleDataAndWorkspace(url, workspaceName) {


this seems like util functions but they are tests. if this helper functions fail does the entire test fail?

if so this seems wrong. we shouldn't use E2E testing if we are doing test setup. it's too slow and flaky. we should use the saved objects api which workspaces supports and just execute queries.

we don't use this one. it is for easy migration.

kavilla · 2024-12-18T01:48:12Z

package.json

@@ -86,7 +86,7 @@
    "release_note:generate": "scripts/use_node scripts/generate_release_note",
    "cypress:run-without-security": "env TZ=America/Los_Angeles NO_COLOR=1 cypress run --env SECURITY_ENABLED=false",
    "cypress:run-with-security": "env TZ=America/Los_Angeles NO_COLOR=1 cypress run --env SECURITY_ENABLED=true,openSearchUrl=https://localhost:9200,WAIT_FOR_LOADER_BUFFER_MS=500",
-    "osd:ciGroup10": "echo \"cypress/integration/apps/query_enhancements/queries.spec.js,cypress/integration/apps/query_enhancements/dataset_selector.spec.js\"",
+    "osd:ciGroup10": "echo \"cypress/integration/core-opensearch-dashboards/opensearch-dashboards/apps/query_enhancements/a_check.spec.js\"",


so we are not gunna execute the queries.spec and dataset_selector.spec?

explained below

kavilla · 2024-12-18T01:48:39Z

cypress/support/e2e.js

+
+import { TestFixtureHandler } from '../lib/test-fixture-handler';
+
+Cypress.Commands.add('setupTestData', (mappingFiles, dataFiles) => {


shouldn't this be in the utils/commands

kavilla · 2024-12-18T01:49:18Z

cypress/lib/test-fixture-handler.js

did the linter not catch this file name? i think we are suppose to use _

kavilla

blocking because this will no longer run the existing tests. it just runs the check spec file

ananzh · 2024-12-18T03:06:19Z

blocking because this will no longer run the existing tests. it just runs the check spec file

I think this is expected. The two tests are written when we haven't enable workspace. We will modify these two tests:

cypress/integration/apps/query_enhancements/dataset_selector.spec.js
cypress/integration/apps/query_enhancements/queries.spec.js

enable workspace 2) modify utilities to use endpoint 3) migrate test lib to main (TestFixtureHandler and MiscUtils)

Signed-off-by: Anan <[email protected]>

tests will be re-added.

Follow up on opensearch-project#9048 by updating data and utilities

1ce8fe6

Feature branch PRs: opensearch-project#9038 opensearch-project#9006 Signed-off-by: Anan <[email protected]>

github-actions bot added distinguished-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Dec 16, 2024

LDrago27 reviewed Dec 16, 2024

View reviewed changes

LDrago27 force-pushed the follow-up-9048 branch from 1ce8fe6 to b7f4609 Compare December 16, 2024 20:19

LDrago27 reviewed Dec 16, 2024

View reviewed changes

kavilla reviewed Dec 18, 2024

View reviewed changes

cypress/scripts/generate_data/index.js

Copy link

Member

kavilla Dec 18, 2024

Choose a reason for hiding this comment

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

what javascript?

kavilla reviewed Dec 18, 2024

View reviewed changes

kavilla previously requested changes Dec 18, 2024

View reviewed changes

LDrago27 previously approved these changes Dec 18, 2024

View reviewed changes

ananzh dismissed LDrago27’s stale review via 290c3cb December 18, 2024 03:34

ananzh force-pushed the follow-up-9048 branch from 290c3cb to 454b7e3 Compare December 18, 2024 03:46

fix PR comments

6fa6cb6

Signed-off-by: Anan <[email protected]>

ananzh force-pushed the follow-up-9048 branch from 454b7e3 to 6fa6cb6 Compare December 18, 2024 03:51

ananzh mentioned this pull request Dec 18, 2024

[Discover 2.0 Testing] TestId-69: Filtering #9047

Open

skip tests before enable workspace

474c2e5

Signed-off-by: Anan <[email protected]>

ananzh force-pushed the follow-up-9048 branch from 0636882 to 474c2e5 Compare December 18, 2024 15:39

AMoo-Miki approved these changes Dec 18, 2024

View reviewed changes

LDrago27 approved these changes Dec 18, 2024

View reviewed changes

ananzh merged commit f9c463d into opensearch-project:main Dec 18, 2024
69 checks passed

ananzh added the discover_2.0-test Issues that are specific to the Discover 2.0 testing initiative label Dec 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow up on #9048 by updating data and utilities #9060

Follow up on #9048 by updating data and utilities #9060

ananzh commented Dec 16, 2024 •

edited

Loading

codecov bot commented Dec 16, 2024 •

edited

Loading

LDrago27 Dec 16, 2024 •

edited

Loading

ananzh Dec 17, 2024

LDrago27 Dec 16, 2024

ananzh Dec 17, 2024

AMoo-Miki Dec 17, 2024 •

edited

Loading

kavilla Dec 18, 2024

kavilla Dec 18, 2024

kavilla Dec 18, 2024

kavilla Dec 18, 2024

kavilla Dec 18, 2024 •

edited

Loading

kavilla Dec 18, 2024

kavilla Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla Dec 18, 2024

ananzh Dec 18, 2024

kavilla left a comment •

edited

Loading

ananzh commented Dec 18, 2024 •

edited

Loading


		import { TestFixtureHandler } from '../lib/test-fixture-handler';

		Cypress.Commands.add('setupTestData', (mappingFiles, dataFiles) => {

Follow up on #9048 by updating data and utilities #9060

Follow up on #9048 by updating data and utilities #9060

Conversation

ananzh commented Dec 16, 2024 • edited Loading

Description

To Run

Issues Resolved

Changelog

Check List

codecov bot commented Dec 16, 2024 • edited Loading

Codecov Report

LDrago27 Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AMoo-Miki Dec 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla left a comment • edited Loading

Choose a reason for hiding this comment

ananzh commented Dec 18, 2024 • edited Loading

ananzh commented Dec 16, 2024 •

edited

Loading

codecov bot commented Dec 16, 2024 •

edited

Loading

LDrago27 Dec 16, 2024 •

edited

Loading

AMoo-Miki Dec 17, 2024 •

edited

Loading

kavilla Dec 18, 2024 •

edited

Loading

kavilla left a comment •

edited

Loading

ananzh commented Dec 18, 2024 •

edited

Loading