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

Simplify api response info for FF tests #1664

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

ohltyler
Copy link
Member

@ohltyler ohltyler commented Dec 4, 2024

Description

Some low-level checks on the responses from ingest/search caused some tests to fail after opensearch-project/dashboards-flow-framework#509

This PR simplifies the logic checking. How responses are rendered may soon change, which would require further changes later. Removing the lower-level checks to prevent flakiness and minimize test rewriting.

Check List

  • Commits are signed per the DCO using --signoff

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: Tyler Ohlsen <[email protected]>
Signed-off-by: Tyler Ohlsen <[email protected]>
@@ -132,20 +132,8 @@ describe('Creating Workflows Using Various Methods', () => {
.should('be.visible')
.click();
cy.fixture(FF_FIXTURE_BASE_PATH + 'semantic_search/ingest_response').then(
Copy link
Contributor

Choose a reason for hiding this comment

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

Tyler, In the fixture please remove filesemantic_search/ingest_response. As it is no longer used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Prefer to leave actually, so we're mocking correctly. Also, it actually is still required, as once ingest responses are returned, we have logic to to open up and auto-switch tabs to ingest or search, so the visibility check is still valid.

In the future, we may add some more flexible logic to parse out the responses and ensure there are visible parts in them as well.

Copy link
Contributor

@saimedhi saimedhi left a comment

Choose a reason for hiding this comment

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

Please make the changes requested. Thank you.

@saimedhi
Copy link
Contributor

saimedhi commented Dec 5, 2024

@ohltyler ohltyler merged commit c835903 into opensearch-project:main Dec 9, 2024
40 of 41 checks passed
@ohltyler ohltyler deleted the fixes branch December 9, 2024 16:46
opensearch-trigger-bot bot pushed a commit that referenced this pull request Dec 9, 2024
Signed-off-by: Tyler Ohlsen <[email protected]>
(cherry picked from commit c835903)
ohltyler added a commit that referenced this pull request Dec 9, 2024
Signed-off-by: Tyler Ohlsen <[email protected]>
(cherry picked from commit c835903)

Co-authored-by: Tyler Ohlsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants