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

[Tests] Fix integration test due to code change in osd-plugin-helper #318

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Apr 27, 2021

Signed-off-by: Ubuntu ubuntu@ip-172-31-24-239.us-west-2.compute.internal

Description

Due to code change in #312 on packages/osd-plugin-helpers/src/cli.ts , packages/osd-plugin-helpers/src/integration_tests/build.test.ts does not work. This PR fixed this particular integration test.

Issues Resolved

na

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

@ananzh ananzh added bug Something isn't working test:integration labels Apr 27, 2021
@odfe-release-bot
Copy link

✅   DCO Check Passed b5c3474

@@ -94,8 +94,7 @@ it('builds a generated plugin into a viable archive', async () => {
);

expect(buildProc.all).toMatchInlineSnapshot(`
" warn These tools might work with 7.9 versions, but there are known workarounds required. See https://github.com/elastic/kibana/issues/82466 for more info
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure that we want to remove this warning and link?

Copy link
Member Author

Choose a reason for hiding this comment

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

In packages/osd-plugin-helpers/src/cli.ts we removed version check so there is no log warning 'These tools might work with 7.9 versions, but there are known workarounds required'. That is the reason this particular functional test failed. Thanks for checking.

Copy link
Member

Choose a reason for hiding this comment

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

this should be auto updated with yarn run test:jest_integration -u

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. yarn run test:jest_integration -u will generate same result by deleting that warning sentence. Mine also changed version 7.9.0 to 1.0.0, since we will change the versions eventually.

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
@odfe-release-bot
Copy link

✅   DCO Check Passed 7b23765

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Smoke test, unit test, integ test, and func test passing.

So is prior to this PR there was a failing integration test running yarn test:jest_integration? Do we need plugins within plugins/ for the test to fail?

LGTM regardless.

Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM !! Thanks @ananzh for taking stab it.

@mihirsoni mihirsoni merged commit 4b73f88 into opensearch-project:main Apr 28, 2021
kavilla pushed a commit that referenced this pull request May 21, 2021
…er (#318)

Signed-off-by: Anan Zhuang <ananzh@amazon.com>

Co-authored-by: Ubuntu <ubuntu@ip-172-31-24-239.us-west-2.compute.internal>
@ananzh ananzh deleted the fix_integration branch June 21, 2021 01:01
SuZhou-Joe added a commit to SuZhou-Joe/OpenSearch-Dashboards that referenced this pull request Apr 7, 2024
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working test:integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants