From 266123224d4fc19eefdac2b085f6c81dff07432b Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 12 Oct 2022 15:24:19 -0400 Subject: [PATCH 01/26] initial commit for sql workbench windows and macos Signed-off-by: Derek Ho --- .../sql-workbench-test-and-build-workflow.yml | 101 +++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sql-workbench-test-and-build-workflow.yml b/.github/workflows/sql-workbench-test-and-build-workflow.yml index 7c0e2549e7..d9322feafa 100644 --- a/.github/workflows/sql-workbench-test-and-build-workflow.yml +++ b/.github/workflows/sql-workbench-test-and-build-workflow.yml @@ -15,7 +15,7 @@ env: OPENSEARCH_PLUGIN_VERSION: 2.4.0.0 jobs: - build: + linux-build: runs-on: ubuntu-latest steps: - name: Checkout Plugin @@ -70,3 +70,102 @@ jobs: with: name: workbench path: ../OpenSearch-Dashboards/plugins/workbench/build + + windows-build: + runs-on: windows-latest + steps: + - name: Checkout Plugin + uses: actions/checkout@v3 + # Enable longer filenames for windows + - name: Enable longer filenames + run: git config --system core.longpaths true + + - name: Checkout OpenSearch Dashboards + uses: actions/checkout@v1 # can't update to v3 because `setup-node` fails + with: + repository: opensearch-project/Opensearch-Dashboards + ref: ${{ env.OPENSEARCH_VERSION }} + path: OpenSearch-Dashboards + + - name: Setup Node + uses: actions/setup-node@v3 + with: + node-version-file: "../OpenSearch-Dashboards/.nvmrc" + registry-url: 'https://registry.npmjs.org' + + - name: Move Workbench to Plugins Dir + run: | + mv workbench ../OpenSearch-Dashboards/plugins + + - name: OpenSearch Dashboards Plugin Bootstrap + uses: nick-fields/retry@v2 + with: + timeout_minutes: 60 + max_attempts: 3 + command: cd ../OpenSearch-Dashboards/plugins/workbench; yarn osd bootstrap + + - name: Test + run: | + cd ../OpenSearch-Dashboards/plugins/workbench + yarn test:jest + + - name: Build Artifact + run: | + cd ../OpenSearch-Dashboards/plugins/workbench + yarn build + mv ./build/*.zip ./build/${{ env.PLUGIN_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}.zip + + - name: Upload Artifact + if: always() + uses: actions/upload-artifact@v1 # can't update to v3 because upload fails + with: + name: workbench + path: ../OpenSearch-Dashboards/plugins/workbench/build + + macos-build: + runs-on: macos-latest + steps: + - name: Checkout Plugin + uses: actions/checkout@v3 + + - name: Checkout OpenSearch Dashboards + uses: actions/checkout@v1 # can't update to v3 because `setup-node` fails + with: + repository: opensearch-project/Opensearch-Dashboards + ref: ${{ env.OPENSEARCH_VERSION }} + path: OpenSearch-Dashboards + + - name: Setup Node + uses: actions/setup-node@v3 + with: + node-version-file: "../OpenSearch-Dashboards/.nvmrc" + registry-url: 'https://registry.npmjs.org' + + - name: Move Workbench to Plugins Dir + run: | + mv workbench ../OpenSearch-Dashboards/plugins + + - name: OpenSearch Dashboards Plugin Bootstrap + uses: nick-fields/retry@v2 + with: + timeout_minutes: 60 + max_attempts: 3 + command: cd ../OpenSearch-Dashboards/plugins/workbench; yarn osd bootstrap + + - name: Test + run: | + cd ../OpenSearch-Dashboards/plugins/workbench + yarn test:jest + + - name: Build Artifact + run: | + cd ../OpenSearch-Dashboards/plugins/workbench + yarn build + mv ./build/*.zip ./build/${{ env.PLUGIN_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}.zip + + - name: Upload Artifact + if: always() + uses: actions/upload-artifact@v1 # can't update to v3 because upload fails + with: + name: workbench + path: ../OpenSearch-Dashboards/plugins/workbench/build \ No newline at end of file From 7a1c7aead9bc8b7922cc00064193ebb6298b1122 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 12 Oct 2022 15:29:08 -0400 Subject: [PATCH 02/26] name files Signed-off-by: Derek Ho --- .github/workflows/sql-workbench-test-and-build-workflow.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/sql-workbench-test-and-build-workflow.yml b/.github/workflows/sql-workbench-test-and-build-workflow.yml index d9322feafa..0b5ef5d34c 100644 --- a/.github/workflows/sql-workbench-test-and-build-workflow.yml +++ b/.github/workflows/sql-workbench-test-and-build-workflow.yml @@ -68,7 +68,7 @@ jobs: if: always() uses: actions/upload-artifact@v1 # can't update to v3 because upload fails with: - name: workbench + name: workbench-linux path: ../OpenSearch-Dashboards/plugins/workbench/build windows-build: @@ -119,7 +119,7 @@ jobs: if: always() uses: actions/upload-artifact@v1 # can't update to v3 because upload fails with: - name: workbench + name: workbench-windows path: ../OpenSearch-Dashboards/plugins/workbench/build macos-build: @@ -167,5 +167,5 @@ jobs: if: always() uses: actions/upload-artifact@v1 # can't update to v3 because upload fails with: - name: workbench + name: workbench-macos path: ../OpenSearch-Dashboards/plugins/workbench/build \ No newline at end of file From ef535e9db81425baa1aa32e65d617eaf757a5446 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 11:03:32 -0400 Subject: [PATCH 03/26] clean up code with matrix Signed-off-by: Derek Ho --- .../sql-workbench-test-and-build-workflow.yml | 114 ++---------------- 1 file changed, 11 insertions(+), 103 deletions(-) diff --git a/.github/workflows/sql-workbench-test-and-build-workflow.yml b/.github/workflows/sql-workbench-test-and-build-workflow.yml index 0b5ef5d34c..a80425a1f2 100644 --- a/.github/workflows/sql-workbench-test-and-build-workflow.yml +++ b/.github/workflows/sql-workbench-test-and-build-workflow.yml @@ -15,9 +15,16 @@ env: OPENSEARCH_PLUGIN_VERSION: 2.4.0.0 jobs: - linux-build: - runs-on: ubuntu-latest + build: + strategy: + matrix: + os: [ubuntu-latest, windows-latest-macos-latest] + runs-on: ${{ matrix.os }} steps: + - name: Enable longer filenames + if: ${{ matrix.os == 'windows-latest' }} + run: git config --system core.longpaths true + - name: Checkout Plugin uses: actions/checkout@v3 @@ -51,7 +58,7 @@ jobs: yarn test:jest --coverage - name: Upload coverage - if: always() + if: ${{ matrix.os == 'ubuntu-latest' }} uses: codecov/codecov-action@v3 with: flags: query-workbench @@ -68,104 +75,5 @@ jobs: if: always() uses: actions/upload-artifact@v1 # can't update to v3 because upload fails with: - name: workbench-linux - path: ../OpenSearch-Dashboards/plugins/workbench/build - - windows-build: - runs-on: windows-latest - steps: - - name: Checkout Plugin - uses: actions/checkout@v3 - # Enable longer filenames for windows - - name: Enable longer filenames - run: git config --system core.longpaths true - - - name: Checkout OpenSearch Dashboards - uses: actions/checkout@v1 # can't update to v3 because `setup-node` fails - with: - repository: opensearch-project/Opensearch-Dashboards - ref: ${{ env.OPENSEARCH_VERSION }} - path: OpenSearch-Dashboards - - - name: Setup Node - uses: actions/setup-node@v3 - with: - node-version-file: "../OpenSearch-Dashboards/.nvmrc" - registry-url: 'https://registry.npmjs.org' - - - name: Move Workbench to Plugins Dir - run: | - mv workbench ../OpenSearch-Dashboards/plugins - - - name: OpenSearch Dashboards Plugin Bootstrap - uses: nick-fields/retry@v2 - with: - timeout_minutes: 60 - max_attempts: 3 - command: cd ../OpenSearch-Dashboards/plugins/workbench; yarn osd bootstrap - - - name: Test - run: | - cd ../OpenSearch-Dashboards/plugins/workbench - yarn test:jest - - - name: Build Artifact - run: | - cd ../OpenSearch-Dashboards/plugins/workbench - yarn build - mv ./build/*.zip ./build/${{ env.PLUGIN_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}.zip - - - name: Upload Artifact - if: always() - uses: actions/upload-artifact@v1 # can't update to v3 because upload fails - with: - name: workbench-windows - path: ../OpenSearch-Dashboards/plugins/workbench/build - - macos-build: - runs-on: macos-latest - steps: - - name: Checkout Plugin - uses: actions/checkout@v3 - - - name: Checkout OpenSearch Dashboards - uses: actions/checkout@v1 # can't update to v3 because `setup-node` fails - with: - repository: opensearch-project/Opensearch-Dashboards - ref: ${{ env.OPENSEARCH_VERSION }} - path: OpenSearch-Dashboards - - - name: Setup Node - uses: actions/setup-node@v3 - with: - node-version-file: "../OpenSearch-Dashboards/.nvmrc" - registry-url: 'https://registry.npmjs.org' - - - name: Move Workbench to Plugins Dir - run: | - mv workbench ../OpenSearch-Dashboards/plugins - - - name: OpenSearch Dashboards Plugin Bootstrap - uses: nick-fields/retry@v2 - with: - timeout_minutes: 60 - max_attempts: 3 - command: cd ../OpenSearch-Dashboards/plugins/workbench; yarn osd bootstrap - - - name: Test - run: | - cd ../OpenSearch-Dashboards/plugins/workbench - yarn test:jest - - - name: Build Artifact - run: | - cd ../OpenSearch-Dashboards/plugins/workbench - yarn build - mv ./build/*.zip ./build/${{ env.PLUGIN_NAME }}-${{ env.OPENSEARCH_PLUGIN_VERSION }}.zip - - - name: Upload Artifact - if: always() - uses: actions/upload-artifact@v1 # can't update to v3 because upload fails - with: - name: workbench-macos + name: workbench-${{ matrix.os }} path: ../OpenSearch-Dashboards/plugins/workbench/build \ No newline at end of file From 8a58eb9299d2420bd7319d3af2b4cea80e09e1a7 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 11:06:14 -0400 Subject: [PATCH 04/26] forgot comma Signed-off-by: Derek Ho --- .github/workflows/sql-workbench-test-and-build-workflow.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sql-workbench-test-and-build-workflow.yml b/.github/workflows/sql-workbench-test-and-build-workflow.yml index a80425a1f2..3878872512 100644 --- a/.github/workflows/sql-workbench-test-and-build-workflow.yml +++ b/.github/workflows/sql-workbench-test-and-build-workflow.yml @@ -18,7 +18,7 @@ jobs: build: strategy: matrix: - os: [ubuntu-latest, windows-latest-macos-latest] + os: [ubuntu-latest, windows-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - name: Enable longer filenames From 828101c062f54660797e526e09dd488c97f8e4b3 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 16:22:00 -0400 Subject: [PATCH 05/26] use cross env for windows and enable windows and macos ci in sql Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 12 ++++++------ workbench/package.json | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index b70092713e..3b8fd1b81e 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -21,10 +21,9 @@ jobs: build: strategy: matrix: - java: - - 11 - - 17 - runs-on: ubuntu-latest + java: [11, 17] + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 @@ -39,6 +38,7 @@ jobs: run: ./gradlew --continue build assemble - name: Run backward compatibility tests + if: ${{ matrix.os == 'ubuntu-latest' }} run: ./scripts/bwctest.sh - name: Create Artifact Path @@ -48,7 +48,7 @@ jobs: # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action - name: Upload SQL Coverage Report - if: always() + if: ${{ matrix.os == 'ubuntu-latest' }} uses: codecov/codecov-action@v3 with: flags: sql-engine @@ -61,7 +61,7 @@ jobs: path: opensearch-sql-builds - name: Upload test reports - if: always() + if: ${{ matrix.os == 'ubuntu-latest' }} uses: actions/upload-artifact@v2 with: name: test-reports diff --git a/workbench/package.json b/workbench/package.json index aebf11b285..9a9b9706f6 100644 --- a/workbench/package.json +++ b/workbench/package.json @@ -16,7 +16,7 @@ "start": "plugin-helpers start", "test:server": "plugin-helpers test:server", "test:browser": "plugin-helpers test:browser", - "test:jest": "NODE_PATH=../../node_modules ../../node_modules/.bin/jest --config ./test/jest.config.js", + "test:jest": "cross-env NODE_PATH=../../node_modules ../../node_modules/.bin/jest --config ./test/jest.config.js", "build": "yarn plugin_helpers build", "plugin_helpers": "node ../../scripts/plugin_helpers" }, @@ -28,6 +28,7 @@ "@testing-library/user-event": "^13.1.9", "@types/hapi-latest": "npm:@types/hapi@18.0.3", "@types/react-router-dom": "^5.3.2", + "cross-env": "7.0.3", "cypress": "^5.0.0", "eslint": "^6.8.0", "eslint-plugin-no-unsanitized": "^3.0.2", From 53ef534870c8636b0d31caaf951a95dde0d3bf21 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 16:41:07 -0400 Subject: [PATCH 06/26] disable integration and jacoco for windows and mac Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 3b8fd1b81e..014c71a400 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -19,10 +19,17 @@ on: jobs: build: + env: + BUILD_ARGS: ${{ matrix.os_build_args }} strategy: matrix: java: [11, 17] os: [ubuntu-latest, windows-latest, macos-latest] + include: + - os: windows-latest + os_build_args: -x integTest -x jacocoTestReport + - os: macos-latest + os_build_args: -x integTest -x jacocoTestReport runs-on: ${{ matrix.os }} steps: @@ -35,7 +42,7 @@ jobs: java-version: ${{ matrix.java }} - name: Build with Gradle - run: ./gradlew --continue build assemble + run: ./gradlew --continue build assemble ${{ env.BUILD_ARGS }} - name: Run backward compatibility tests if: ${{ matrix.os == 'ubuntu-latest' }} From 7569d6d7cfe03d35660da1aec31e41582c80eb1f Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 16:55:51 -0400 Subject: [PATCH 07/26] add gitattributes file to normalize line endings Signed-off-by: Derek Ho --- .gitattributes | 1 + .github/workflows/sql-test-and-build-workflow.yml | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..176a458f94 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text=auto diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 014c71a400..b5b5a35819 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -19,17 +19,12 @@ on: jobs: build: - env: - BUILD_ARGS: ${{ matrix.os_build_args }} strategy: + # Run all jobs + fail-fast: false matrix: java: [11, 17] os: [ubuntu-latest, windows-latest, macos-latest] - include: - - os: windows-latest - os_build_args: -x integTest -x jacocoTestReport - - os: macos-latest - os_build_args: -x integTest -x jacocoTestReport runs-on: ${{ matrix.os }} steps: @@ -42,7 +37,7 @@ jobs: java-version: ${{ matrix.java }} - name: Build with Gradle - run: ./gradlew --continue build assemble ${{ env.BUILD_ARGS }} + run: ./gradlew --continue build assemble - name: Run backward compatibility tests if: ${{ matrix.os == 'ubuntu-latest' }} From 912731b8edafaaaa6b79400e7213d2565f4a1933 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 17:06:05 -0400 Subject: [PATCH 08/26] move gitattributes into appropriate folder Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 2 +- .gitattributes => sql/.gitattributes | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename .gitattributes => sql/.gitattributes (100%) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index b5b5a35819..f0b9b2705b 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -59,7 +59,7 @@ jobs: - name: Upload Artifacts uses: actions/upload-artifact@v2 with: - name: opensearch-sql + name: opensearch-sql-${{ matrix.os }} path: opensearch-sql-builds - name: Upload test reports diff --git a/.gitattributes b/sql/.gitattributes similarity index 100% rename from .gitattributes rename to sql/.gitattributes From 6a6cfd6602afe2b752fd1e7950c832bac79f9598 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 13 Oct 2022 17:09:16 -0400 Subject: [PATCH 09/26] configure line endings for windows Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index f0b9b2705b..f6c0ed142f 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -29,6 +29,10 @@ jobs: steps: - uses: actions/checkout@v3 + + - name: Change line endings for windows + if: ${{ matrix.os == 'windows-latest' }} + run: git config --global core.autocrlf false - name: Set up JDK ${{ matrix.java }} uses: actions/setup-java@v3 From 0d016efe7b7e40373a89365746596e528c739295 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 14 Oct 2022 17:15:58 -0400 Subject: [PATCH 10/26] fix one test file and get rid of git attributes Signed-off-by: Derek Ho --- .../format/RawResponseFormatterTest.java | 38 +++++++++---------- sql/.gitattributes | 1 - 2 files changed, 19 insertions(+), 20 deletions(-) delete mode 100644 sql/.gitattributes diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java index 87d2d6f57f..d8e06f81ab 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java @@ -36,7 +36,7 @@ void formatResponse() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("name", "John", "age", 20)), tupleValue(ImmutableMap.of("name", "Smith", "age", 30)))); - String expected = "name|age\nJohn|20\nSmith|30"; + String expected = "name|age%nJohn|20%nSmith|30"; assertEquals(expected, rawFormater.format(response)); } @@ -50,7 +50,7 @@ void sanitizeHeaders() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of( "=firstname", "John", "+lastname", "Smith", "-city", "Seattle", "@age", 20)))); - String expected = "=firstname|+lastname|-city|@age\n" + String expected = "=firstname|+lastname|-city|@age%n" + "John|Smith|Seattle|20"; assertEquals(expected, rawFormater.format(response)); } @@ -66,12 +66,12 @@ void sanitizeData() { tupleValue(ImmutableMap.of("city", "-Seattle")), tupleValue(ImmutableMap.of("city", "@Seattle")), tupleValue(ImmutableMap.of("city", "Seattle=")))); - String expected = "city\n" - + "Seattle\n" - + "=Seattle\n" - + "+Seattle\n" - + "-Seattle\n" - + "@Seattle\n" + String expected = "city%n" + + "Seattle%n" + + "=Seattle%n" + + "+Seattle%n" + + "-Seattle%n" + + "@Seattle%n" + "Seattle="; assertEquals(expected, rawFormater.format(response)); } @@ -83,7 +83,7 @@ void quoteIfRequired() { new ExecutionEngine.Schema.Column("||age", "||age", INTEGER))); QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("na|me", "John|Smith", "||age", "30|||")))); - String expected = "\"na|me\"|\"||age\"\n" + String expected = "\"na|me\"|\"||age\"%n" + "\"John|Smith\"|\"30|||\""; assertEquals(expected, rawFormater.format(response)); } @@ -92,7 +92,7 @@ void quoteIfRequired() { void formatError() { Throwable t = new RuntimeException("This is an exception"); String expected = - "{\n \"type\": \"RuntimeException\",\n \"reason\": \"This is an exception\"\n}"; + "{%n \"type\": \"RuntimeException\",%n \"reason\": \"This is an exception\"%n}"; assertEquals(expected, rawFormater.format(t)); } @@ -104,8 +104,8 @@ void escapeSanitize() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("city", "=Seattle")), tupleValue(ImmutableMap.of("city", "||Seattle")))); - String expected = "city\n" - + "=Seattle\n" + String expected = "city%n" + + "=Seattle%n" + "\"||Seattle\""; assertEquals(expected, escapeFormatter.format(response)); } @@ -117,8 +117,8 @@ void senstiveCharater() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("city", "@Seattle")), tupleValue(ImmutableMap.of("city", "++Seattle")))); - String expected = "city\n" - + "@Seattle\n" + String expected = "city%n" + + "@Seattle%n" + "++Seattle"; assertEquals(expected, rawFormater.format(response)); } @@ -131,8 +131,8 @@ void senstiveCharaterWithSanitize() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("city", "@Seattle")), tupleValue(ImmutableMap.of("city", "++Seattle|||")))); - String expected = "city\n" - + "@Seattle\n" + String expected = "city%n" + + "@Seattle%n" + "\"++Seattle|||\""; assertEquals(expected, testFormater.format(response)); } @@ -148,9 +148,9 @@ void replaceNullValues() { ImmutableMap.of("firstname", LITERAL_NULL, "city", stringValue("Seattle"))), ExprTupleValue.fromExprValueMap( ImmutableMap.of("firstname", stringValue("John"), "city", LITERAL_MISSING)))); - String expected = "name|city\n" - + "John|Seattle\n" - + "|Seattle\n" + String expected = "name|city%n" + + "John|Seattle%n" + + "|Seattle%n" + "John|"; assertEquals(expected, rawFormater.format(response)); } diff --git a/sql/.gitattributes b/sql/.gitattributes deleted file mode 100644 index 176a458f94..0000000000 --- a/sql/.gitattributes +++ /dev/null @@ -1 +0,0 @@ -* text=auto From a8709d2a885354a76f56a8104376ca3e1392ac39 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 14 Oct 2022 17:17:34 -0400 Subject: [PATCH 11/26] disable doctest and integ test Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index f6c0ed142f..d467741260 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -19,12 +19,17 @@ on: jobs: build: + env: + BUILD_ARGS: ${{ matrix.os_build_args }} strategy: # Run all jobs fail-fast: false matrix: java: [11, 17] os: [ubuntu-latest, windows-latest, macos-latest] + include: + - os: windows-latest + os_build_args: -x doctest -x integTest -x jacocoTestReport runs-on: ${{ matrix.os }} steps: @@ -41,7 +46,7 @@ jobs: java-version: ${{ matrix.java }} - name: Build with Gradle - run: ./gradlew --continue build assemble + run: ./gradlew --continue build assemble ${{ env.BUILD_ARGS }} - name: Run backward compatibility tests if: ${{ matrix.os == 'ubuntu-latest' }} From d4c9f1e0f80a7d2495fbd896d2ac09baece2b3a7 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 10:09:30 -0400 Subject: [PATCH 12/26] fix up tests Signed-off-by: Derek Ho --- .../workflows/sql-test-and-build-workflow.yml | 10 +- .../AggregationQueryBuilderTest.java | 693 +++++++++--------- .../dsl/MetricAggregationBuilderTest.java | 247 +++---- .../response/format/ErrorFormatter.java | 6 +- .../format/CsvResponseFormatterTest.java | 45 +- .../response/format/ErrorFormatterTest.java | 3 +- .../format/RawResponseFormatterTest.java | 17 +- .../SimpleJsonResponseFormatterTest.java | 61 +- 8 files changed, 544 insertions(+), 538 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index d467741260..057e4ee98b 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -29,15 +29,13 @@ jobs: os: [ubuntu-latest, windows-latest, macos-latest] include: - os: windows-latest - os_build_args: -x doctest -x integTest -x jacocoTestReport + os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc + - os: macos-latest + os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 - - - name: Change line endings for windows - if: ${{ matrix.os == 'windows-latest' }} - run: git config --global core.autocrlf false - name: Set up JDK ${{ matrix.java }} uses: actions/setup-java@v3 @@ -46,7 +44,7 @@ jobs: java-version: ${{ matrix.java }} - name: Build with Gradle - run: ./gradlew --continue build assemble ${{ env.BUILD_ARGS }} + run: ./gradlew --continue build ${{ env.BUILD_ARGS }} - name: Run backward compatibility tests if: ${{ matrix.os == 'ubuntu-latest' }} diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilderTest.java index 04aedc0f01..3614d82e59 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilderTest.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; +import static org.opensearch.sql.common.utils.StringUtils.format; import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; @@ -72,31 +73,31 @@ void set_up() { @Test void should_build_composite_aggregation_for_field_reference() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"name\" : {\n" - + " \"terms\" : {\n" - + " \"field\" : \"name\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(age)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"name\" : {%n" + + " \"terms\" : {%n" + + " \"field\" : \"name\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(age)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("avg(age)", new AvgAggregator(Arrays.asList(ref("age", INTEGER)), INTEGER))), @@ -105,31 +106,31 @@ void should_build_composite_aggregation_for_field_reference() { @Test void should_build_composite_aggregation_for_field_reference_with_order() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"name\" : {\n" - + " \"terms\" : {\n" - + " \"field\" : \"name\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"last\",\n" - + " \"order\" : \"desc\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(age)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"name\" : {%n" + + " \"terms\" : {%n" + + " \"field\" : \"name\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"last\",%n" + + " \"order\" : \"desc\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(age)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("avg(age)", new AvgAggregator(Arrays.asList(ref("age", INTEGER)), INTEGER))), @@ -152,31 +153,31 @@ void should_build_type_mapping_for_field_reference() { @Test void should_build_composite_aggregation_for_field_reference_of_keyword() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"name\" : {\n" - + " \"terms\" : {\n" - + " \"field\" : \"name.keyword\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(age)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"name\" : {%n" + + " \"terms\" : {%n" + + " \"field\" : \"name.keyword\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(age)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("avg(age)", new AvgAggregator(Arrays.asList(ref("age", INTEGER)), INTEGER))), @@ -201,37 +202,37 @@ void should_build_composite_aggregation_for_expression() { Expression expr = invocation.getArgument(0); return expr.toString(); }).when(serializer).serialize(any()); - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"age\" : {\n" - + " \"terms\" : {\n" - + " \"script\" : {\n" - + " \"source\" : \"asin(age)\",\n" - + " \"lang\" : \"opensearch_query_expression\"\n" - + " },\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(balance)\" : {\n" - + " \"avg\" : {\n" - + " \"script\" : {\n" - + " \"source\" : \"abs(balance)\",\n" - + " \"lang\" : \"opensearch_query_expression\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"age\" : {%n" + + " \"terms\" : {%n" + + " \"script\" : {%n" + + " \"source\" : \"asin(age)\",%n" + + " \"lang\" : \"opensearch_query_expression\"%n" + + " },%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(balance)\" : {%n" + + " \"avg\" : {%n" + + " \"script\" : {%n" + + " \"source\" : \"abs(balance)\",%n" + + " \"lang\" : \"opensearch_query_expression\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("avg(balance)", new AvgAggregator( @@ -241,40 +242,40 @@ void should_build_composite_aggregation_for_expression() { @Test void should_build_composite_aggregation_follow_with_order_by_position() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"name\" : {\n" - + " \"terms\" : {\n" - + " \"field\" : \"name\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"last\",\n" - + " \"order\" : \"desc\"\n" - + " }\n" - + " }\n" - + " }, {\n" - + " \"age\" : {\n" - + " \"terms\" : {\n" - + " \"field\" : \"age\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(balance)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"balance\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"name\" : {%n" + + " \"terms\" : {%n" + + " \"field\" : \"name\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"last\",%n" + + " \"order\" : \"desc\"%n" + + " }%n" + + " }%n" + + " }, {%n" + + " \"age\" : {%n" + + " \"terms\" : {%n" + + " \"field\" : \"age\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(balance)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"balance\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( agg(named("avg(balance)", avg(ref("balance", INTEGER), INTEGER))), group(named("age", ref("age", INTEGER)), named("name", ref("name", STRING))), @@ -298,14 +299,14 @@ void should_build_type_mapping_for_expression() { @Test void should_build_aggregation_without_bucket() { - assertEquals( - "{\n" - + " \"avg(balance)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"balance\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"avg(balance)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"balance\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("avg(balance)", new AvgAggregator( @@ -315,29 +316,29 @@ void should_build_aggregation_without_bucket() { @Test void should_build_filter_aggregation() { - assertEquals( - "{\n" - + " \"avg(age) filter(where age > 34)\" : {\n" - + " \"filter\" : {\n" - + " \"range\" : {\n" - + " \"age\" : {\n" - + " \"from\" : 20,\n" - + " \"to\" : null,\n" - + " \"include_lower\" : false,\n" - + " \"include_upper\" : true,\n" - + " \"boost\" : 1.0\n" - + " }\n" - + " }\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(age) filter(where age > 34)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"avg(age) filter(where age > 34)\" : {%n" + + " \"filter\" : {%n" + + " \"range\" : {%n" + + " \"age\" : {%n" + + " \"from\" : 20,%n" + + " \"to\" : null,%n" + + " \"include_lower\" : false,%n" + + " \"include_upper\" : true,%n" + + " \"boost\" : 1.0%n" + + " }%n" + + " }%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(age) filter(where age > 34)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList(named("avg(age) filter(where age > 34)", new AvgAggregator(Arrays.asList(ref("age", INTEGER)), INTEGER) @@ -347,46 +348,46 @@ void should_build_filter_aggregation() { @Test void should_build_filter_aggregation_group_by() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"gender\" : {\n" - + " \"terms\" : {\n" - + " \"field\" : \"gender\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(age) filter(where age > 34)\" : {\n" - + " \"filter\" : {\n" - + " \"range\" : {\n" - + " \"age\" : {\n" - + " \"from\" : 20,\n" - + " \"to\" : null,\n" - + " \"include_lower\" : false,\n" - + " \"include_upper\" : true,\n" - + " \"boost\" : 1.0\n" - + " }\n" - + " }\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"avg(age) filter(where age > 34)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"gender\" : {%n" + + " \"terms\" : {%n" + + " \"field\" : \"gender\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(age) filter(where age > 34)\" : {%n" + + " \"filter\" : {%n" + + " \"range\" : {%n" + + " \"age\" : {%n" + + " \"from\" : 20,%n" + + " \"to\" : null,%n" + + " \"include_lower\" : false,%n" + + " \"include_upper\" : true,%n" + + " \"boost\" : 1.0%n" + + " }%n" + + " }%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"avg(age) filter(where age > 34)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList(named("avg(age) filter(where age > 34)", new AvgAggregator(Arrays.asList(ref("age", INTEGER)), INTEGER) @@ -408,32 +409,32 @@ void should_build_type_mapping_without_bucket() { @Test void should_build_histogram() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"SpanExpression(field=age, value=10, unit=NONE)\" : {\n" - + " \"histogram\" : {\n" - + " \"field\" : \"age\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\",\n" - + " \"interval\" : 10.0\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"count(a)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"a\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"SpanExpression(field=age, value=10, unit=NONE)\" : {%n" + + " \"histogram\" : {%n" + + " \"field\" : \"age\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\",%n" + + " \"interval\" : 10.0%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"count(a)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"a\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(a)", new CountAggregator(Arrays.asList(ref("a", INTEGER)), INTEGER))), @@ -442,37 +443,37 @@ void should_build_histogram() { @Test void should_build_histogram_two_metrics() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"SpanExpression(field=age, value=10, unit=NONE)\" : {\n" - + " \"histogram\" : {\n" - + " \"field\" : \"age\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\",\n" - + " \"interval\" : 10.0\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"count(a)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"a\"\n" - + " }\n" - + " },\n" - + " \"avg(b)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"b\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"SpanExpression(field=age, value=10, unit=NONE)\" : {%n" + + " \"histogram\" : {%n" + + " \"field\" : \"age\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\",%n" + + " \"interval\" : 10.0%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"count(a)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"a\"%n" + + " }%n" + + " },%n" + + " \"avg(b)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"b\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(a)", new CountAggregator(Arrays.asList(ref("a", INTEGER)), INTEGER)), @@ -482,32 +483,32 @@ void should_build_histogram_two_metrics() { @Test void fixed_interval_time_span() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"SpanExpression(field=timestamp, value=1, unit=H)\" : {\n" - + " \"date_histogram\" : {\n" - + " \"field\" : \"timestamp\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\",\n" - + " \"fixed_interval\" : \"1h\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"count(a)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"a\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"SpanExpression(field=timestamp, value=1, unit=H)\" : {%n" + + " \"date_histogram\" : {%n" + + " \"field\" : \"timestamp\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\",%n" + + " \"fixed_interval\" : \"1h\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"count(a)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"a\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(a)", new CountAggregator(Arrays.asList(ref("a", INTEGER)), INTEGER))), @@ -516,32 +517,32 @@ void fixed_interval_time_span() { @Test void calendar_interval_time_span() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"SpanExpression(field=date, value=1, unit=W)\" : {\n" - + " \"date_histogram\" : {\n" - + " \"field\" : \"date\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\",\n" - + " \"calendar_interval\" : \"1w\"\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"count(a)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"a\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"SpanExpression(field=date, value=1, unit=W)\" : {%n" + + " \"date_histogram\" : {%n" + + " \"field\" : \"date\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\",%n" + + " \"calendar_interval\" : \"1w\"%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"count(a)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"a\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(a)", new CountAggregator(Arrays.asList(ref("a", INTEGER)), INTEGER))), @@ -550,32 +551,32 @@ void calendar_interval_time_span() { @Test void general_span() { - assertEquals( - "{\n" - + " \"composite_buckets\" : {\n" - + " \"composite\" : {\n" - + " \"size\" : 1000,\n" - + " \"sources\" : [ {\n" - + " \"SpanExpression(field=age, value=1, unit=NONE)\" : {\n" - + " \"histogram\" : {\n" - + " \"field\" : \"age\",\n" - + " \"missing_bucket\" : true,\n" - + " \"missing_order\" : \"first\",\n" - + " \"order\" : \"asc\",\n" - + " \"interval\" : 1.0\n" - + " }\n" - + " }\n" - + " } ]\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"count(a)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"a\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"composite_buckets\" : {%n" + + " \"composite\" : {%n" + + " \"size\" : 1000,%n" + + " \"sources\" : [ {%n" + + " \"SpanExpression(field=age, value=1, unit=NONE)\" : {%n" + + " \"histogram\" : {%n" + + " \"field\" : \"age\",%n" + + " \"missing_bucket\" : true,%n" + + " \"missing_order\" : \"first\",%n" + + " \"order\" : \"asc\",%n" + + " \"interval\" : 1.0%n" + + " }%n" + + " }%n" + + " } ]%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"count(a)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"a\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(a)", new CountAggregator(Arrays.asList(ref("a", INTEGER)), INTEGER))), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/MetricAggregationBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/MetricAggregationBuilderTest.java index 845e32ba83..5161b35021 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/MetricAggregationBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/aggregation/dsl/MetricAggregationBuilderTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.when; +import static org.opensearch.sql.common.utils.StringUtils.format; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.expression.DSL.literal; @@ -62,14 +63,14 @@ void set_up() { @Test void should_build_avg_aggregation() { - assertEquals( - "{\n" - + " \"avg(age)\" : {\n" - + " \"avg\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"avg(age)\" : {%n" + + " \"avg\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("avg(age)", @@ -78,14 +79,14 @@ void should_build_avg_aggregation() { @Test void should_build_sum_aggregation() { - assertEquals( - "{\n" - + " \"sum(age)\" : {\n" - + " \"sum\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"sum(age)\" : {%n" + + " \"sum\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("sum(age)", @@ -94,14 +95,14 @@ void should_build_sum_aggregation() { @Test void should_build_count_aggregation() { - assertEquals( - "{\n" - + " \"count(age)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"count(age)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(age)", @@ -110,14 +111,14 @@ void should_build_count_aggregation() { @Test void should_build_count_star_aggregation() { - assertEquals( - "{\n" - + " \"count(*)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"_index\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"count(*)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"_index\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(*)", @@ -126,14 +127,14 @@ void should_build_count_star_aggregation() { @Test void should_build_count_other_literal_aggregation() { - assertEquals( - "{\n" - + " \"count(1)\" : {\n" - + " \"value_count\" : {\n" - + " \"field\" : \"_index\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"count(1)\" : {%n" + + " \"value_count\" : {%n" + + " \"field\" : \"_index\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("count(1)", @@ -142,14 +143,14 @@ void should_build_count_other_literal_aggregation() { @Test void should_build_min_aggregation() { - assertEquals( - "{\n" - + " \"min(age)\" : {\n" - + " \"min\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"min(age)\" : {%n" + + " \"min\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("min(age)", @@ -158,14 +159,14 @@ void should_build_min_aggregation() { @Test void should_build_max_aggregation() { - assertEquals( - "{\n" - + " \"max(age)\" : {\n" - + " \"max\" : {\n" - + " \"field\" : \"age\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"max(age)\" : {%n" + + " \"max\" : {%n" + + " \"field\" : \"age\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("max(age)", @@ -174,15 +175,15 @@ void should_build_max_aggregation() { @Test void should_build_varPop_aggregation() { - assertEquals( - "{\n" - + " \"var_pop(age)\" : {\n" - + " \"extended_stats\" : {\n" - + " \"field\" : \"age\",\n" - + " \"sigma\" : 2.0\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"var_pop(age)\" : {%n" + + " \"extended_stats\" : {%n" + + " \"field\" : \"age\",%n" + + " \"sigma\" : 2.0%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("var_pop(age)", @@ -191,15 +192,15 @@ void should_build_varPop_aggregation() { @Test void should_build_varSamp_aggregation() { - assertEquals( - "{\n" - + " \"var_samp(age)\" : {\n" - + " \"extended_stats\" : {\n" - + " \"field\" : \"age\",\n" - + " \"sigma\" : 2.0\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"var_samp(age)\" : {%n" + + " \"extended_stats\" : {%n" + + " \"field\" : \"age\",%n" + + " \"sigma\" : 2.0%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("var_samp(age)", @@ -208,15 +209,15 @@ void should_build_varSamp_aggregation() { @Test void should_build_stddevPop_aggregation() { - assertEquals( - "{\n" - + " \"stddev_pop(age)\" : {\n" - + " \"extended_stats\" : {\n" - + " \"field\" : \"age\",\n" - + " \"sigma\" : 2.0\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"stddev_pop(age)\" : {%n" + + " \"extended_stats\" : {%n" + + " \"field\" : \"age\",%n" + + " \"sigma\" : 2.0%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("stddev_pop(age)", @@ -225,15 +226,15 @@ void should_build_stddevPop_aggregation() { @Test void should_build_stddevSamp_aggregation() { - assertEquals( - "{\n" - + " \"stddev_samp(age)\" : {\n" - + " \"extended_stats\" : {\n" - + " \"field\" : \"age\",\n" - + " \"sigma\" : 2.0\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"stddev_samp(age)\" : {%n" + + " \"extended_stats\" : {%n" + + " \"field\" : \"age\",%n" + + " \"sigma\" : 2.0%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Arrays.asList( named("stddev_samp(age)", @@ -242,14 +243,14 @@ void should_build_stddevSamp_aggregation() { @Test void should_build_cardinality_aggregation() { - assertEquals( - "{\n" - + " \"count(distinct name)\" : {\n" - + " \"cardinality\" : {\n" - + " \"field\" : \"name\"\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"count(distinct name)\" : {%n" + + " \"cardinality\" : {%n" + + " \"field\" : \"name\"%n" + + " }%n" + + " }%n" + + "}"), buildQuery( Collections.singletonList(named("count(distinct name)", new CountAggregator( Collections.singletonList(ref("name", STRING)), INTEGER).distinct(true))))); @@ -257,29 +258,29 @@ void should_build_cardinality_aggregation() { @Test void should_build_filtered_cardinality_aggregation() { - assertEquals( - "{\n" - + " \"count(distinct name) filter(where age > 30)\" : {\n" - + " \"filter\" : {\n" - + " \"range\" : {\n" - + " \"age\" : {\n" - + " \"from\" : 30,\n" - + " \"to\" : null,\n" - + " \"include_lower\" : false,\n" - + " \"include_upper\" : true,\n" - + " \"boost\" : 1.0\n" - + " }\n" - + " }\n" - + " },\n" - + " \"aggregations\" : {\n" - + " \"count(distinct name) filter(where age > 30)\" : {\n" - + " \"cardinality\" : {\n" - + " \"field\" : \"name\"\n" - + " }\n" - + " }\n" - + " }\n" - + " }\n" - + "}", + assertEquals(format( + "{%n" + + " \"count(distinct name) filter(where age > 30)\" : {%n" + + " \"filter\" : {%n" + + " \"range\" : {%n" + + " \"age\" : {%n" + + " \"from\" : 30,%n" + + " \"to\" : null,%n" + + " \"include_lower\" : false,%n" + + " \"include_upper\" : true,%n" + + " \"boost\" : 1.0%n" + + " }%n" + + " }%n" + + " },%n" + + " \"aggregations\" : {%n" + + " \"count(distinct name) filter(where age > 30)\" : {%n" + + " \"cardinality\" : {%n" + + " \"field\" : \"name\"%n" + + " }%n" + + " }%n" + + " }%n" + + " }%n" + + "}"), buildQuery(Collections.singletonList(named( "count(distinct name) filter(where age > 30)", new CountAggregator(Collections.singletonList(ref("name", STRING)), INTEGER) diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java index 40848e959b..e21e731a2d 100644 --- a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java +++ b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java @@ -45,12 +45,14 @@ public static String prettyFormat(Throwable t) { public static String compactJsonify(Object jsonObject) { return AccessController.doPrivileged( - (PrivilegedAction) () -> GSON.toJson(jsonObject)); + (PrivilegedAction) () -> GSON.toJson(jsonObject) + .replaceAll("\\n|\\r\\n", System.getProperty("line.separator"))); } public static String prettyJsonify(Object jsonObject) { return AccessController.doPrivileged( - (PrivilegedAction) () -> PRETTY_PRINT_GSON.toJson(jsonObject)); + (PrivilegedAction) () -> PRETTY_PRINT_GSON.toJson(jsonObject) + .replaceAll("\\n|\\r\\n", System.getProperty("line.separator"))); } @RequiredArgsConstructor diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java index 8998086afc..475cb30946 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java @@ -7,6 +7,7 @@ package org.opensearch.sql.protocol.response.format; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opensearch.sql.common.utils.StringUtils.format; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; @@ -37,8 +38,8 @@ void formatResponse() { tupleValue(ImmutableMap.of("name", "John", "age", 20)), tupleValue(ImmutableMap.of("name", "Smith", "age", 30)))); CsvResponseFormatter formatter = new CsvResponseFormatter(); - String expected = "name,age\nJohn,20\nSmith,30"; - assertEquals(expected, formatter.format(response)); + String expected = "name,age%nJohn,20%nSmith,30"; + assertEquals(format(expected), formatter.format(response)); } @Test @@ -51,9 +52,9 @@ void sanitizeHeaders() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of( "=firstname", "John", "+lastname", "Smith", "-city", "Seattle", "@age", 20)))); - String expected = "'=firstname,'+lastname,'-city,'@age\n" + String expected = "'=firstname,'+lastname,'-city,'@age%n" + "John,Smith,Seattle,20"; - assertEquals(expected, formatter.format(response)); + assertEquals(format(expected), formatter.format(response)); } @Test @@ -67,14 +68,14 @@ void sanitizeData() { tupleValue(ImmutableMap.of("city", "-Seattle")), tupleValue(ImmutableMap.of("city", "@Seattle")), tupleValue(ImmutableMap.of("city", "Seattle=")))); - String expected = "city\n" - + "Seattle\n" - + "'=Seattle\n" - + "'+Seattle\n" - + "'-Seattle\n" - + "'@Seattle\n" + String expected = "city%n" + + "Seattle%n" + + "'=Seattle%n" + + "'+Seattle%n" + + "'-Seattle%n" + + "'@Seattle%n" + "Seattle="; - assertEquals(expected, formatter.format(response)); + assertEquals(format(expected), formatter.format(response)); } @Test @@ -84,17 +85,17 @@ void quoteIfRequired() { new ExecutionEngine.Schema.Column(",,age", ",,age", INTEGER))); QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("na,me", "John,Smith", ",,age", "30,,,")))); - String expected = "\"na,me\",\",,age\"\n" + String expected = "\"na,me\",\",,age\"%n" + "\"John,Smith\",\"30,,,\""; - assertEquals(expected, formatter.format(response)); + assertEquals(format(expected), formatter.format(response)); } @Test void formatError() { Throwable t = new RuntimeException("This is an exception"); String expected = - "{\n \"type\": \"RuntimeException\",\n \"reason\": \"This is an exception\"\n}"; - assertEquals(expected, formatter.format(t)); + "{%n \"type\": \"RuntimeException\",%n \"reason\": \"This is an exception\"%n}"; + assertEquals(format(expected), formatter.format(t)); } @Test @@ -105,10 +106,10 @@ void escapeSanitize() { QueryResult response = new QueryResult(schema, Arrays.asList( tupleValue(ImmutableMap.of("city", "=Seattle")), tupleValue(ImmutableMap.of("city", ",,Seattle")))); - String expected = "city\n" - + "=Seattle\n" + String expected = "city%n" + + "=Seattle%n" + "\",,Seattle\""; - assertEquals(expected, escapeFormatter.format(response)); + assertEquals(format(expected), escapeFormatter.format(response)); } @Test @@ -122,11 +123,11 @@ void replaceNullValues() { ImmutableMap.of("firstname", LITERAL_NULL, "city", stringValue("Seattle"))), ExprTupleValue.fromExprValueMap( ImmutableMap.of("firstname", stringValue("John"), "city", LITERAL_MISSING)))); - String expected = "name,city\n" - + "John,Seattle\n" - + ",Seattle\n" + String expected = "name,city%n" + + "John,Seattle%n" + + ",Seattle%n" + "John,"; - assertEquals(expected, formatter.format(response)); + assertEquals(format(expected), formatter.format(response)); } } diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java index f19f6bb2ae..a8e5f6dee5 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java @@ -6,6 +6,7 @@ package org.opensearch.sql.protocol.response.format; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opensearch.sql.common.utils.StringUtils.format; import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; @@ -16,7 +17,7 @@ class ErrorFormatterTest { @Test void htmlEscaping_should_disabled() { assertEquals( - "{\n" + " \"request\": \"index=test\"\n" + "}", + format("{%n" + " \"request\": \"index=test\"%n" + "}"), ErrorFormatter.prettyJsonify(ImmutableMap.of("request", "index=test"))); assertEquals( "{\"request\":\"index=test\"}", diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java index d8e06f81ab..12cdc4bf96 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java @@ -7,6 +7,7 @@ package org.opensearch.sql.protocol.response.format; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opensearch.sql.common.utils.StringUtils.format; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; @@ -37,7 +38,7 @@ void formatResponse() { tupleValue(ImmutableMap.of("name", "John", "age", 20)), tupleValue(ImmutableMap.of("name", "Smith", "age", 30)))); String expected = "name|age%nJohn|20%nSmith|30"; - assertEquals(expected, rawFormater.format(response)); + assertEquals(format(expected), rawFormater.format(response)); } @Test @@ -52,7 +53,7 @@ void sanitizeHeaders() { "=firstname", "John", "+lastname", "Smith", "-city", "Seattle", "@age", 20)))); String expected = "=firstname|+lastname|-city|@age%n" + "John|Smith|Seattle|20"; - assertEquals(expected, rawFormater.format(response)); + assertEquals(format(expected), rawFormater.format(response)); } @Test @@ -73,7 +74,7 @@ void sanitizeData() { + "-Seattle%n" + "@Seattle%n" + "Seattle="; - assertEquals(expected, rawFormater.format(response)); + assertEquals(format(expected), rawFormater.format(response)); } @Test @@ -85,7 +86,7 @@ void quoteIfRequired() { tupleValue(ImmutableMap.of("na|me", "John|Smith", "||age", "30|||")))); String expected = "\"na|me\"|\"||age\"%n" + "\"John|Smith\"|\"30|||\""; - assertEquals(expected, rawFormater.format(response)); + assertEquals(format(expected), rawFormater.format(response)); } @Test @@ -93,7 +94,7 @@ void formatError() { Throwable t = new RuntimeException("This is an exception"); String expected = "{%n \"type\": \"RuntimeException\",%n \"reason\": \"This is an exception\"%n}"; - assertEquals(expected, rawFormater.format(t)); + assertEquals(format(expected), rawFormater.format(t)); } @Test @@ -120,7 +121,7 @@ void senstiveCharater() { String expected = "city%n" + "@Seattle%n" + "++Seattle"; - assertEquals(expected, rawFormater.format(response)); + assertEquals(format(expected), rawFormater.format(response)); } @Test @@ -134,7 +135,7 @@ void senstiveCharaterWithSanitize() { String expected = "city%n" + "@Seattle%n" + "\"++Seattle|||\""; - assertEquals(expected, testFormater.format(response)); + assertEquals(format(expected), testFormater.format(response)); } @Test @@ -152,7 +153,7 @@ void replaceNullValues() { + "John|Seattle%n" + "|Seattle%n" + "John|"; - assertEquals(expected, rawFormater.format(response)); + assertEquals(format(expected), rawFormater.format(response)); } } diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index 8b4438cf91..eb5d5f5393 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -7,6 +7,7 @@ package org.opensearch.sql.protocol.response.format; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opensearch.sql.common.utils.StringUtils.format; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; import static org.opensearch.sql.data.model.ExprValueUtils.tupleValue; @@ -54,31 +55,31 @@ void formatResponsePretty() { tupleValue(ImmutableMap.of("firstname", "John", "age", 20)), tupleValue(ImmutableMap.of("firstname", "Smith", "age", 30)))); SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(PRETTY); - assertEquals( - "{\n" - + " \"schema\": [\n" - + " {\n" - + " \"name\": \"firstname\",\n" - + " \"type\": \"string\"\n" - + " },\n" - + " {\n" - + " \"name\": \"age\",\n" - + " \"type\": \"integer\"\n" - + " }\n" - + " ],\n" - + " \"datarows\": [\n" - + " [\n" - + " \"John\",\n" - + " 20\n" - + " ],\n" - + " [\n" - + " \"Smith\",\n" - + " 30\n" - + " ]\n" - + " ],\n" - + " \"total\": 2,\n" - + " \"size\": 2\n" - + "}", + assertEquals(format( + "{%n" + + " \"schema\": [%n" + + " {%n" + + " \"name\": \"firstname\",%n" + + " \"type\": \"string\"%n" + + " },%n" + + " {%n" + + " \"name\": \"age\",%n" + + " \"type\": \"integer\"%n" + + " }%n" + + " ],%n" + + " \"datarows\": [%n" + + " [%n" + + " \"John\",%n" + + " 20%n" + + " ],%n" + + " [%n" + + " \"Smith\",%n" + + " 30%n" + + " ]%n" + + " ],%n" + + " \"total\": 2,%n" + + " \"size\": 2%n" + + "}"), formatter.format(response)); } @@ -165,11 +166,11 @@ void formatError() { @Test void formatErrorPretty() { SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(PRETTY); - assertEquals( - "{\n" - + " \"type\": \"RuntimeException\",\n" - + " \"reason\": \"This is an exception\"\n" - + "}", + assertEquals(format( + "{%n" + + " \"type\": \"RuntimeException\",%n" + + " \"reason\": \"This is an exception\"%n" + + "}"), formatter.format(new RuntimeException("This is an exception"))); } } From 9df2acd6e65f4dd0ca59a806cfe8de047f343033 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 10:22:08 -0400 Subject: [PATCH 13/26] fix tests and add java docs Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 3 +++ .../sql/protocol/response/format/ErrorFormatter.java | 6 ++++++ .../protocol/response/format/RawResponseFormatterTest.java | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 057e4ee98b..320ba7046e 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -35,6 +35,9 @@ jobs: runs-on: ${{ matrix.os }} steps: + - name: Change line endings for windows + if: ${{ matrix.os == 'windows-latest' }} + run: git config --global core.autocrlf input - uses: actions/checkout@v3 - name: Set up JDK ${{ matrix.java }} diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java index e21e731a2d..64a1cd842f 100644 --- a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java +++ b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java @@ -43,12 +43,18 @@ public static String prettyFormat(Throwable t) { return prettyJsonify(error); } + /** + * Util method to format {@link Object} to string in compact printing. + */ public static String compactJsonify(Object jsonObject) { return AccessController.doPrivileged( (PrivilegedAction) () -> GSON.toJson(jsonObject) .replaceAll("\\n|\\r\\n", System.getProperty("line.separator"))); } + /** + * Util method to format {@link Object} to string in pretty printing. + */ public static String prettyJsonify(Object jsonObject) { return AccessController.doPrivileged( (PrivilegedAction) () -> PRETTY_PRINT_GSON.toJson(jsonObject) diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java index 12cdc4bf96..8fe7ee1006 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java @@ -108,7 +108,7 @@ void escapeSanitize() { String expected = "city%n" + "=Seattle%n" + "\"||Seattle\""; - assertEquals(expected, escapeFormatter.format(response)); + assertEquals(format(expected), escapeFormatter.format(response)); } @Test From 8f98fa4cdd24ee0ebb13e7e71d0f08f1d99eb2e7 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 13:53:30 -0400 Subject: [PATCH 14/26] revert error format Signed-off-by: Derek Ho --- .../sql/protocol/response/format/ErrorFormatter.java | 12 ++---------- .../response/format/CsvResponseFormatterTest.java | 4 ++-- .../protocol/response/format/ErrorFormatterTest.java | 2 +- .../response/format/RawResponseFormatterTest.java | 4 ++-- .../format/SimpleJsonResponseFormatterTest.java | 10 +++++----- 5 files changed, 12 insertions(+), 20 deletions(-) diff --git a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java index 64a1cd842f..40848e959b 100644 --- a/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java +++ b/protocol/src/main/java/org/opensearch/sql/protocol/response/format/ErrorFormatter.java @@ -43,22 +43,14 @@ public static String prettyFormat(Throwable t) { return prettyJsonify(error); } - /** - * Util method to format {@link Object} to string in compact printing. - */ public static String compactJsonify(Object jsonObject) { return AccessController.doPrivileged( - (PrivilegedAction) () -> GSON.toJson(jsonObject) - .replaceAll("\\n|\\r\\n", System.getProperty("line.separator"))); + (PrivilegedAction) () -> GSON.toJson(jsonObject)); } - /** - * Util method to format {@link Object} to string in pretty printing. - */ public static String prettyJsonify(Object jsonObject) { return AccessController.doPrivileged( - (PrivilegedAction) () -> PRETTY_PRINT_GSON.toJson(jsonObject) - .replaceAll("\\n|\\r\\n", System.getProperty("line.separator"))); + (PrivilegedAction) () -> PRETTY_PRINT_GSON.toJson(jsonObject)); } @RequiredArgsConstructor diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java index 475cb30946..7008b51fa6 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/CsvResponseFormatterTest.java @@ -94,8 +94,8 @@ void quoteIfRequired() { void formatError() { Throwable t = new RuntimeException("This is an exception"); String expected = - "{%n \"type\": \"RuntimeException\",%n \"reason\": \"This is an exception\"%n}"; - assertEquals(format(expected), formatter.format(t)); + "{\n \"type\": \"RuntimeException\",\n \"reason\": \"This is an exception\"\n}"; + assertEquals(expected, formatter.format(t)); } @Test diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java index a8e5f6dee5..02714a7ffb 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java @@ -17,7 +17,7 @@ class ErrorFormatterTest { @Test void htmlEscaping_should_disabled() { assertEquals( - format("{%n" + " \"request\": \"index=test\"%n" + "}"), + format("{\n" + " \"request\": \"index=test\"\n" + "}"), ErrorFormatter.prettyJsonify(ImmutableMap.of("request", "index=test"))); assertEquals( "{\"request\":\"index=test\"}", diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java index 8fe7ee1006..24b5a4431d 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/RawResponseFormatterTest.java @@ -93,8 +93,8 @@ void quoteIfRequired() { void formatError() { Throwable t = new RuntimeException("This is an exception"); String expected = - "{%n \"type\": \"RuntimeException\",%n \"reason\": \"This is an exception\"%n}"; - assertEquals(format(expected), rawFormater.format(t)); + "{\n \"type\": \"RuntimeException\",\n \"reason\": \"This is an exception\"\n}"; + assertEquals(expected, rawFormater.format(t)); } @Test diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index eb5d5f5393..8be080a816 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -166,11 +166,11 @@ void formatError() { @Test void formatErrorPretty() { SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(PRETTY); - assertEquals(format( - "{%n" - + " \"type\": \"RuntimeException\",%n" - + " \"reason\": \"This is an exception\"%n" - + "}"), + assertEquals( + "{\n" + + " \"type\": \"RuntimeException\",\n" + + " \"reason\": \"This is an exception\"\n" + + "}", formatter.format(new RuntimeException("This is an exception"))); } } From 7a39dcb7be2e67dd58f9b81666be13b191119d15 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 14:09:51 -0400 Subject: [PATCH 15/26] revert pretty format response Signed-off-by: Derek Ho --- .../SimpleJsonResponseFormatterTest.java | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index 8be080a816..8f47618807 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -55,31 +55,31 @@ void formatResponsePretty() { tupleValue(ImmutableMap.of("firstname", "John", "age", 20)), tupleValue(ImmutableMap.of("firstname", "Smith", "age", 30)))); SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(PRETTY); - assertEquals(format( - "{%n" - + " \"schema\": [%n" - + " {%n" - + " \"name\": \"firstname\",%n" - + " \"type\": \"string\"%n" - + " },%n" - + " {%n" - + " \"name\": \"age\",%n" - + " \"type\": \"integer\"%n" - + " }%n" - + " ],%n" - + " \"datarows\": [%n" - + " [%n" - + " \"John\",%n" - + " 20%n" - + " ],%n" - + " [%n" - + " \"Smith\",%n" - + " 30%n" - + " ]%n" - + " ],%n" - + " \"total\": 2,%n" - + " \"size\": 2%n" - + "}"), + assertEquals( + "{\n" + + " \"schema\": [\n" + + " {\n" + + " \"name\": \"firstname\",\n" + + " \"type\": \"string\"\n" + + " },\n" + + " {\n" + + " \"name\": \"age\",\n" + + " \"type\": \"integer\"\n" + + " }\n" + + " ],\n" + + " \"datarows\": [\n" + + " [\n" + + " \"John\",\n" + + " 20\n" + + " ],\n" + + " [\n" + + " \"Smith\",\n" + + " 30\n" + + " ]\n" + + " ],\n" + + " \"total\": 2,\n" + + " \"size\": 2\n" + + "}", formatter.format(response)); } From bd8572a7314be83f79e86b4d3eae1a7ae45719a7 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 14:43:33 -0400 Subject: [PATCH 16/26] revert error formatter test file Signed-off-by: Derek Ho --- .../sql/protocol/response/format/ErrorFormatterTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java index 02714a7ffb..f19f6bb2ae 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/ErrorFormatterTest.java @@ -6,7 +6,6 @@ package org.opensearch.sql.protocol.response.format; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.opensearch.sql.common.utils.StringUtils.format; import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; @@ -17,7 +16,7 @@ class ErrorFormatterTest { @Test void htmlEscaping_should_disabled() { assertEquals( - format("{\n" + " \"request\": \"index=test\"\n" + "}"), + "{\n" + " \"request\": \"index=test\"\n" + "}", ErrorFormatter.prettyJsonify(ImmutableMap.of("request", "index=test"))); assertEquals( "{\"request\":\"index=test\"}", From a2b473b27d7d69ee3c687f8bcc90b15412f16a1b Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 21:02:50 -0400 Subject: [PATCH 17/26] replace carriage return with nothing Signed-off-by: Derek Ho --- .../sql/legacy/unittest/utils/PrettyFormatterTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/PrettyFormatterTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/PrettyFormatterTest.java index fc20d818e6..f876b14110 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/PrettyFormatterTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/PrettyFormatterTest.java @@ -31,11 +31,13 @@ public void assertFormatterWithoutContentInside() throws IOException { public void assertFormatterOutputsPrettyJson() throws IOException { String explainFormattedPrettyFilePath = TestUtils.getResourceFilePath( "/src/test/resources/expectedOutput/explain_format_pretty.json"); - String explainFormattedPretty = Files.toString(new File(explainFormattedPrettyFilePath), StandardCharsets.UTF_8); + String explainFormattedPretty = Files.toString(new File(explainFormattedPrettyFilePath), StandardCharsets.UTF_8) + .replaceAll("\r", ""); String explainFormattedOnelineFilePath = TestUtils.getResourceFilePath( "/src/test/resources/explain_format_oneline.json"); - String explainFormattedOneline = Files.toString(new File(explainFormattedOnelineFilePath), StandardCharsets.UTF_8); + String explainFormattedOneline = Files.toString(new File(explainFormattedOnelineFilePath), StandardCharsets.UTF_8) + .replaceAll("\r", ""); String result = JsonPrettyFormatter.format(explainFormattedOneline); assertThat(result, equalTo(explainFormattedPretty)); From a3747a372343804b72d04ca3d263831cef855203 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 17 Oct 2022 21:07:49 -0400 Subject: [PATCH 18/26] remove windows git config Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 320ba7046e..057e4ee98b 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -35,9 +35,6 @@ jobs: runs-on: ${{ matrix.os }} steps: - - name: Change line endings for windows - if: ${{ matrix.os == 'windows-latest' }} - run: git config --global core.autocrlf input - uses: actions/checkout@v3 - name: Set up JDK ${{ matrix.java }} From e70df11d2d6de9350c4c005ba1c1cc312da610d6 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 19 Oct 2022 17:08:00 -0400 Subject: [PATCH 19/26] fix PR comments and fail test on purpose to see if upload test reports succeeds Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 13 +++++++++---- .../sql-workbench-test-and-build-workflow.yml | 3 ++- .../format/SimpleJsonResponseFormatterTest.java | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 057e4ee98b..52a30afa44 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -25,8 +25,13 @@ jobs: # Run all jobs fail-fast: false matrix: - java: [11, 17] - os: [ubuntu-latest, windows-latest, macos-latest] + entry: + - { os: ubuntu-latest, java: 11 } + - { os: windows-latest, java: 11 } + - { os: macos-latest, java: 11 } + - { os: ubuntu-latest, java: 17 } + - { os: windows-latest, java: 17 } + - { os: macos-latest, java: 17 } include: - os: windows-latest os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc @@ -57,7 +62,7 @@ jobs: # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action - name: Upload SQL Coverage Report - if: ${{ matrix.os == 'ubuntu-latest' }} + if: ${{ always() && matrix.os == 'ubuntu-latest' }} uses: codecov/codecov-action@v3 with: flags: sql-engine @@ -70,7 +75,7 @@ jobs: path: opensearch-sql-builds - name: Upload test reports - if: ${{ matrix.os == 'ubuntu-latest' }} + if: ${{ always() && matrix.os == 'ubuntu-latest' }} uses: actions/upload-artifact@v2 with: name: test-reports diff --git a/.github/workflows/sql-workbench-test-and-build-workflow.yml b/.github/workflows/sql-workbench-test-and-build-workflow.yml index 3878872512..e5f52065b6 100644 --- a/.github/workflows/sql-workbench-test-and-build-workflow.yml +++ b/.github/workflows/sql-workbench-test-and-build-workflow.yml @@ -76,4 +76,5 @@ jobs: uses: actions/upload-artifact@v1 # can't update to v3 because upload fails with: name: workbench-${{ matrix.os }} - path: ../OpenSearch-Dashboards/plugins/workbench/build \ No newline at end of file + path: ../OpenSearch-Dashboards/plugins/workbench/build + \ No newline at end of file diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index 8f47618807..ce8dd3e92c 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -40,7 +40,7 @@ void formatResponse() { tupleValue(ImmutableMap.of("firstname", "Smith", "age", 30)))); SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(COMPACT); assertEquals( - "{\"schema\":[{\"name\":\"firstname\",\"type\":\"string\"}," + "{\"schema\":[{\"name\":\"firstname\",\"type\":\"string\"},a;woehgi;whaeg" + "{\"name\":\"age\",\"type\":\"integer\"}],\"datarows\":" + "[[\"John\",20],[\"Smith\",30]],\"total\":2,\"size\":2}", formatter.format(response)); From de65184a74ca54460951de1cfb3f3c9053053cb8 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 19 Oct 2022 17:13:43 -0400 Subject: [PATCH 20/26] fix matrix entry Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 52a30afa44..46103b0057 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -46,13 +46,13 @@ jobs: uses: actions/setup-java@v3 with: distribution: 'temurin' - java-version: ${{ matrix.java }} + java-version: ${{ matrix.entry.java }} - name: Build with Gradle run: ./gradlew --continue build ${{ env.BUILD_ARGS }} - name: Run backward compatibility tests - if: ${{ matrix.os == 'ubuntu-latest' }} + if: ${{ matrix.entry.os == 'ubuntu-latest' }} run: ./scripts/bwctest.sh - name: Create Artifact Path @@ -62,7 +62,7 @@ jobs: # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action - name: Upload SQL Coverage Report - if: ${{ always() && matrix.os == 'ubuntu-latest' }} + if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} uses: codecov/codecov-action@v3 with: flags: sql-engine @@ -71,11 +71,11 @@ jobs: - name: Upload Artifacts uses: actions/upload-artifact@v2 with: - name: opensearch-sql-${{ matrix.os }} + name: opensearch-sql-${{ matrix.entry.os }} path: opensearch-sql-builds - name: Upload test reports - if: ${{ always() && matrix.os == 'ubuntu-latest' }} + if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} uses: actions/upload-artifact@v2 with: name: test-reports From a6f3ceb80745978778cefcdcf1b5a4d201ff0431 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 19 Oct 2022 17:23:44 -0400 Subject: [PATCH 21/26] remove test failure Signed-off-by: Derek Ho --- .../response/format/SimpleJsonResponseFormatterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index ce8dd3e92c..8f47618807 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -40,7 +40,7 @@ void formatResponse() { tupleValue(ImmutableMap.of("firstname", "Smith", "age", 30)))); SimpleJsonResponseFormatter formatter = new SimpleJsonResponseFormatter(COMPACT); assertEquals( - "{\"schema\":[{\"name\":\"firstname\",\"type\":\"string\"},a;woehgi;whaeg" + "{\"schema\":[{\"name\":\"firstname\",\"type\":\"string\"}," + "{\"name\":\"age\",\"type\":\"integer\"}],\"datarows\":" + "[[\"John\",20],[\"Smith\",30]],\"total\":2,\"size\":2}", formatter.format(response)); From 3f372bba8689b016690fe2d84616b8b2dcb2688c Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 20 Oct 2022 16:50:27 -0400 Subject: [PATCH 22/26] fix up to run on right os Signed-off-by: Derek Ho --- .../workflows/sql-test-and-build-workflow.yml | 105 +++++++++--------- 1 file changed, 50 insertions(+), 55 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 46103b0057..aaedf7c25e 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -20,72 +20,67 @@ on: jobs: build: env: - BUILD_ARGS: ${{ matrix.os_build_args }} + BUILD_ARGS: ${{ matrix.entry.os_build_args }} strategy: # Run all jobs fail-fast: false matrix: entry: - { os: ubuntu-latest, java: 11 } - - { os: windows-latest, java: 11 } - - { os: macos-latest, java: 11 } + - { os: windows-latest, java: 11, os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc} + - { os: macos-latest, java: 11, os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc } - { os: ubuntu-latest, java: 17 } - - { os: windows-latest, java: 17 } - - { os: macos-latest, java: 17 } - include: - - os: windows-latest - os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc - - os: macos-latest - os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc - runs-on: ${{ matrix.os }} + - { os: windows-latest, java: 17, os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc } + - { os: macos-latest, java: 17, os_build_args: -x doctest -x integTest -x jacocoTestReport -x compileJdbc } + runs-on: ${{ matrix.entry.os }} steps: - - uses: actions/checkout@v3 - - - name: Set up JDK ${{ matrix.java }} - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: ${{ matrix.entry.java }} - - - name: Build with Gradle - run: ./gradlew --continue build ${{ env.BUILD_ARGS }} + - uses: actions/checkout@v3 - - name: Run backward compatibility tests - if: ${{ matrix.entry.os == 'ubuntu-latest' }} - run: ./scripts/bwctest.sh + - name: Set up JDK ${{ matrix.java }} + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: ${{ matrix.entry.java }} - - name: Create Artifact Path - run: | - mkdir -p opensearch-sql-builds - cp -r ./plugin/build/distributions/*.zip opensearch-sql-builds/ + - name: Build with Gradle + run: ./gradlew --continue build ${{ env.BUILD_ARGS }} - # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action - - name: Upload SQL Coverage Report - if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} - uses: codecov/codecov-action@v3 - with: - flags: sql-engine - token: ${{ secrets.CODECOV_TOKEN }} + - name: Run backward compatibility tests + if: ${{ matrix.entry.os == 'ubuntu-latest' }} + run: ./scripts/bwctest.sh - - name: Upload Artifacts - uses: actions/upload-artifact@v2 - with: - name: opensearch-sql-${{ matrix.entry.os }} - path: opensearch-sql-builds + - name: Create Artifact Path + run: | + mkdir -p opensearch-sql-builds + cp -r ./plugin/build/distributions/*.zip opensearch-sql-builds/ - - name: Upload test reports - if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} - uses: actions/upload-artifact@v2 - with: - name: test-reports - path: | - sql/build/reports/** - ppl/build/reports/** - core/build/reports/** - common/build/reports/** - opensearch/build/reports/** - integ-test/build/reports/** - protocol/build/reports/** - legacy/build/reports/** - plugin/build/reports/** + # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action + - name: Upload SQL Coverage Report + if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} + uses: codecov/codecov-action@v3 + with: + flags: sql-engine + token: ${{ secrets.CODECOV_TOKEN }} + + - name: Upload Artifacts + uses: actions/upload-artifact@v2 + with: + name: opensearch-sql-${{ matrix.entry.os }} + path: opensearch-sql-builds + + - name: Upload test reports + if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} + uses: actions/upload-artifact@v2 + with: + name: test-reports + path: | + sql/build/reports/** + ppl/build/reports/** + core/build/reports/** + common/build/reports/** + opensearch/build/reports/** + integ-test/build/reports/** + protocol/build/reports/** + legacy/build/reports/** + plugin/build/reports/** From 5e24f3c36b73ddfd5f5bd5708e2ebd008a6742da Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 20 Oct 2022 16:51:36 -0400 Subject: [PATCH 23/26] fix up indentation Signed-off-by: Derek Ho --- .../workflows/sql-test-and-build-workflow.yml | 84 +++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index aaedf7c25e..85714ef795 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -35,52 +35,52 @@ jobs: runs-on: ${{ matrix.entry.os }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v3 - - name: Set up JDK ${{ matrix.java }} - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: ${{ matrix.entry.java }} + - name: Set up JDK ${{ matrix.java }} + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: ${{ matrix.entry.java }} - - name: Build with Gradle - run: ./gradlew --continue build ${{ env.BUILD_ARGS }} + - name: Build with Gradle + run: ./gradlew --continue build ${{ env.BUILD_ARGS }} - - name: Run backward compatibility tests - if: ${{ matrix.entry.os == 'ubuntu-latest' }} - run: ./scripts/bwctest.sh + - name: Run backward compatibility tests + if: ${{ matrix.entry.os == 'ubuntu-latest' }} + run: ./scripts/bwctest.sh - - name: Create Artifact Path - run: | - mkdir -p opensearch-sql-builds - cp -r ./plugin/build/distributions/*.zip opensearch-sql-builds/ + - name: Create Artifact Path + run: | + mkdir -p opensearch-sql-builds + cp -r ./plugin/build/distributions/*.zip opensearch-sql-builds/ - # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action - - name: Upload SQL Coverage Report - if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} - uses: codecov/codecov-action@v3 - with: - flags: sql-engine - token: ${{ secrets.CODECOV_TOKEN }} + # This step uses the codecov-action Github action: https://github.com/codecov/codecov-action + - name: Upload SQL Coverage Report + if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} + uses: codecov/codecov-action@v3 + with: + flags: sql-engine + token: ${{ secrets.CODECOV_TOKEN }} - - name: Upload Artifacts - uses: actions/upload-artifact@v2 - with: - name: opensearch-sql-${{ matrix.entry.os }} - path: opensearch-sql-builds + - name: Upload Artifacts + uses: actions/upload-artifact@v2 + with: + name: opensearch-sql-${{ matrix.entry.os }} + path: opensearch-sql-builds - - name: Upload test reports - if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} - uses: actions/upload-artifact@v2 - with: - name: test-reports - path: | - sql/build/reports/** - ppl/build/reports/** - core/build/reports/** - common/build/reports/** - opensearch/build/reports/** - integ-test/build/reports/** - protocol/build/reports/** - legacy/build/reports/** - plugin/build/reports/** + - name: Upload test reports + if: ${{ always() && matrix.entry.os == 'ubuntu-latest' }} + uses: actions/upload-artifact@v2 + with: + name: test-reports + path: | + sql/build/reports/** + ppl/build/reports/** + core/build/reports/** + common/build/reports/** + opensearch/build/reports/** + integ-test/build/reports/** + protocol/build/reports/** + legacy/build/reports/** + plugin/build/reports/** From 3fbe3e44abcf44f525065e97ce0991068a19a62b Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 20 Oct 2022 17:34:59 -0400 Subject: [PATCH 24/26] remove env. build_args to make it cleaner Signed-off-by: Derek Ho --- .github/workflows/sql-test-and-build-workflow.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml index 85714ef795..3d063a2bfc 100644 --- a/.github/workflows/sql-test-and-build-workflow.yml +++ b/.github/workflows/sql-test-and-build-workflow.yml @@ -19,8 +19,6 @@ on: jobs: build: - env: - BUILD_ARGS: ${{ matrix.entry.os_build_args }} strategy: # Run all jobs fail-fast: false @@ -44,7 +42,7 @@ jobs: java-version: ${{ matrix.entry.java }} - name: Build with Gradle - run: ./gradlew --continue build ${{ env.BUILD_ARGS }} + run: ./gradlew --continue build ${{ matrix.entry.os_build_args }} - name: Run backward compatibility tests if: ${{ matrix.entry.os == 'ubuntu-latest' }} From 954c072e639ba2dc992c48807190e2c0ba5119df Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 20 Oct 2022 17:48:25 -0400 Subject: [PATCH 25/26] try removing env var Signed-off-by: Derek Ho --- workbench/package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/workbench/package.json b/workbench/package.json index 9a9b9706f6..2360dc4d91 100644 --- a/workbench/package.json +++ b/workbench/package.json @@ -16,7 +16,7 @@ "start": "plugin-helpers start", "test:server": "plugin-helpers test:server", "test:browser": "plugin-helpers test:browser", - "test:jest": "cross-env NODE_PATH=../../node_modules ../../node_modules/.bin/jest --config ./test/jest.config.js", + "test:jest": "../../node_modules/.bin/jest --config ./test/jest.config.js", "build": "yarn plugin_helpers build", "plugin_helpers": "node ../../scripts/plugin_helpers" }, @@ -28,7 +28,6 @@ "@testing-library/user-event": "^13.1.9", "@types/hapi-latest": "npm:@types/hapi@18.0.3", "@types/react-router-dom": "^5.3.2", - "cross-env": "7.0.3", "cypress": "^5.0.0", "eslint": "^6.8.0", "eslint-plugin-no-unsanitized": "^3.0.2", From 79f765c4a49ebcd441885a8d74e3353c757d4d8d Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 20 Oct 2022 17:51:05 -0400 Subject: [PATCH 26/26] remove unecessary import Signed-off-by: Derek Ho --- .../response/format/SimpleJsonResponseFormatterTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java index 8f47618807..8b4438cf91 100644 --- a/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java +++ b/protocol/src/test/java/org/opensearch/sql/protocol/response/format/SimpleJsonResponseFormatterTest.java @@ -7,7 +7,6 @@ package org.opensearch.sql.protocol.response.format; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.opensearch.sql.common.utils.StringUtils.format; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; import static org.opensearch.sql.data.model.ExprValueUtils.tupleValue;