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

Fix CI YAML Tests #168

Merged
merged 9 commits into from
May 10, 2023
Merged

Fix CI YAML Tests #168

merged 9 commits into from
May 10, 2023

Conversation

Xtansia
Copy link
Collaborator

@Xtansia Xtansia commented May 10, 2023

Description

This fixes several issues with the YAML tests, including those that are currently failing due to the new unsigned data type. This improves the mechanism by which we convert the request bodies into the json! {} macro, so that we can avoid doing RegEx replaces on JSON strings. It also adds a hack for the version skip logic due to OS versions "resetting" to "1.0" that meant many test cases were being ignored as they were for ES >= 7.0. Additionally adding logic to cleanup index templates in test teardown so that some of the now running tests pass.
This results in ~300-400 extra test cases now running and passing. (Previously ~800 cases, now ~1200 cases)

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.

Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
@Xtansia Xtansia added the skip-changelog Skip changelog verification label May 10, 2023
@dblock
Copy link
Member

dblock commented May 10, 2023

Question: without looking at the root cause, was this expected? will users experience a backwards incompatible change/break?

@codecov-commenter
Copy link

codecov-commenter commented May 10, 2023

Codecov Report

Merging #168 (7c6b7c8) into main (eff1e0f) will increase coverage by 7.71%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
+ Coverage   65.82%   73.54%   +7.71%     
==========================================
  Files         316      398      +82     
  Lines       52719    62613    +9894     
==========================================
+ Hits        34704    46046   +11342     
+ Misses      18015    16567    -1448     
Flag Coverage Δ
integration 73.54% <ø> (+7.71%) ⬆️

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

see 215 files with indirect coverage changes

@Xtansia
Copy link
Collaborator Author

Xtansia commented May 10, 2023

Question: without looking at the root cause, was this expected? will users experience a backwards incompatible change/break?

@dblock Not sure I fully understand your question. The issues here are basically entirely related to some questionable things the yaml_test_runner was doing since pre-fork. So generally unrelated to the user experience

@Xtansia Xtansia merged commit 9383059 into opensearch-project:main May 10, 2023
@Xtansia Xtansia deleted the fix/yaml-tests branch May 10, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skip changelog verification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants