Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added testing to the generated OpenAPI. #286

Closed
wants to merge 18 commits into from

Conversation

dblock
Copy link
Member

@dblock dblock commented May 7, 2024

Description

This adds tests using dredd.

  1. Builds the spec that only includes APIs with x-tested. This is a new feature of the merger than can be extended in the future to exclude deprecated APIs, or ones for a specified hosted target (AOS, AOSS, etc.), or version of OpenSearch.
  2. Starts an OpenSearch docker container.
  3. Runs dredd against it for the APIs present in the specs.

Passing spec.

pass: GET (200) / duration: 90ms
complete: 1 passing, 0 failing, 0 errors, 0 skipped, 1 total
complete: Tests took 15136ms

Failing spec:

fail: GET (200) / duration: 75ms
info: Displaying failed tests...
fail: GET (200) / duration: 75ms
fail: body: At '/version/build_flavor' Missing required property: build_flavor

request: 
method: GET
uri: /
headers: 
    Accept: application/json; charset=UTF-8
    User-Agent: Dredd/14.1.0 (Darwin 23.4.0; arm64)
    Authorization:  Basic YWRtaW46YWRtaW4=

body: 



expected: 
headers: 
    Content-Type: application/json; charset=UTF-8

body: 
{
  "cluster_name": "",
  "cluster_uuid": "",
  "name": "",
  "tagline": "",
  "version": {
    "build_date": "",
    "build_flavor": "",
    "build_hash": "",
    "build_snapshot": false,
    "build_type": "",
    "lucene_version": "",
    "minimum_index_compatibility_version": "",
    "minimum_wire_compatibility_version": "",
    "number": ""
  }
}
statusCode: 200


actual: 
statusCode: 200
headers: 
    content-type: application/json; charset=UTF-8
    content-length: 566

bodyEncoding: utf-8
body: 
{
  "name": "2c8264d2ffb3",
  "cluster_name": "docker-cluster",
  "cluster_uuid": "grah_e-CRTii2hv1unQ1bg",
  "version": {
    "distribution": "opensearch",
    "number": "2.9.0",
    "build_type": "tar",
    "build_hash": "1164221ee2b8ba3560f0ff492309867beea28433",
    "build_date": "2023-07-18T21:22:48.164885046Z",
    "build_snapshot": false,
    "lucene_version": "9.7.0",
    "minimum_wire_compatibility_version": "7.10.0",
    "minimum_index_compatibility_version": "7.0.0"
  },
  "tagline": "The OpenSearch Project: https://opensearch.org/"
}

complete: 0 passing, 1 failing, 0 errors, 0 skipped, 1 total
complete: Tests took 83ms

In order to get GET / to pass we needed to supply an example of a response, otherwise dredd thinks that build_flavor is a required parameter (see apiaryio/dredd#1430).

Issues Resolved

Resolves #277

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

github-actions bot commented May 7, 2024

API specs implemented for 266/649 (40%) APIs.

@dblock dblock force-pushed the dredd branch 2 times, most recently from f9f7ce0 to 4ce3af7 Compare May 7, 2024 16:28
Copy link

github-actions bot commented May 7, 2024

API specs implemented for 266/649 (40%) APIs.

2 similar comments
Copy link

github-actions bot commented May 7, 2024

API specs implemented for 266/649 (40%) APIs.

Copy link

github-actions bot commented May 8, 2024

API specs implemented for 266/649 (40%) APIs.

@dblock
Copy link
Member Author

dblock commented May 8, 2024

The spec is pretty far from passing dredd, but I'll keep chipping at it. @Xtansia @nhtruong LMK what you think about this approach?

Copy link

github-actions bot commented May 9, 2024

API specs implemented for 266/649 (40%) APIs.

Copy link

github-actions bot commented May 9, 2024

API specs implemented for 266/649 (40%) APIs.

Copy link

github-actions bot commented May 9, 2024

API specs implemented for 266/649 (40%) APIs.

Copy link

github-actions bot commented May 9, 2024

API specs implemented for 266/649 (40%) APIs.

1 similar comment
Copy link

github-actions bot commented May 9, 2024

API specs implemented for 266/649 (40%) APIs.

Copy link

API specs implemented for 299/649 (46%) APIs.

Copy link

API specs implemented for 299/649 (46%) APIs.

1 similar comment
Copy link

API specs implemented for 299/649 (46%) APIs.

Copy link

API specs implemented for 299/649 (46%) APIs.

2 similar comments
Copy link

API specs implemented for 299/649 (46%) APIs.

Copy link

API specs implemented for 299/649 (46%) APIs.

Copy link

API specs implemented for 299/649 (46%) APIs.

tools/eslint.config.mjs Outdated Show resolved Hide resolved
@dblock dblock force-pushed the dredd branch 2 times, most recently from ee0fcd3 to c4546f3 Compare May 13, 2024 15:12
@dblock
Copy link
Member Author

dblock commented May 13, 2024

@nhtruong I added POST and DELETE specs, which required correcting the response code from POST to be 201, but now the linter complains that the responses for all index should be the same.

$ npm run lint:spec

> [email protected] lint:spec
> ts-node tools/src/linter/lint.ts

Validating /Users/dblock/source/opensearch-project/opensearch-api-specification/dblock-opensearch-api-specification/spec ...
Errors found:

{
  file: 'namespaces/_core.yaml',
  location: 'Operation Group: index',
  message: "3 'index' operations must have an identical set of responses."
}

Total errors: 1

Help me correct this the right way?

@nhtruong
Copy link
Collaborator

nhtruong commented May 13, 2024

Regarding: https://github.com/opensearch-project/opensearch-api-specification/pull/286#issuecomment-2108698322https://github.com/opensearch-project/opensearch-api-specification/pull/286#issuecomment-2108698322

We'll need to update the linter to accept a list of exceptions OR update this lint rule to consider 200 and 201 to be exchangeable. We can't get rid of this lint because the client generators expect operations of the same group to share certain properties, and responses is an important one.

@Xtansia
Copy link
Collaborator

Xtansia commented May 13, 2024

You can use 2XX as the key to just say it'll be some successful response code but not specific (https://swagger.io/specification/#responses-object), though that loses some fidelity

@dblock
Copy link
Member Author

dblock commented May 30, 2024

Closing in favor of #299

@dblock dblock closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PROPOSAL] Add integration tests
3 participants