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

Replaced emdedded RedisServer with testcontainers. #2221

Merged
merged 6 commits into from
Nov 8, 2021

Conversation

videnkz
Copy link
Contributor

@videnkz videnkz commented Oct 31, 2021

closes #2175

What does this PR do?

Checklist

  • This is something else
    • Replaced emdedded RedisServer with testcontainers( embedded redis server uses 2.8.9 version, I used - 6.2.6. From the context of the tests, there should be no difference.)

…est classes(Lettuce*Test) - junit4 has been replaced with version5
@github-actions github-actions bot added agent-java community Issues and PRs created by the community triage labels Oct 31, 2021
@apmmachine
Copy link
Contributor

apmmachine commented Oct 31, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-11-08T07:14:30.192+0000

  • Duration: 68 min 51 sec

  • Commit: 19dabd5

Test stats 🧪

Test Results
Failed 0
Passed 2510
Skipped 19
Total 2529

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

  • run compatibility tests : Run the JDK Compatibility test.

  • run integration tests : Run the APM-ITs.

@videnkz
Copy link
Contributor Author

videnkz commented Oct 31, 2021

Hi @cachedout , please check this.
This should solve problem(Unless others suddenly appear :D)

@eyalkoren
Copy link
Contributor

run elasticsearch-ci/docs

@eyalkoren eyalkoren requested a review from felixbarny November 1, 2021 14:41
@eyalkoren
Copy link
Contributor

@felixbarny do we need to test a specific Redis version?

Copy link
Member

@felixbarny felixbarny left a comment

Choose a reason for hiding this comment

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

Please rename the tests extending AbstractRedisInstrumentationTest so that they end with IT (integration test). This makes them not run when doing a mvn test, only a mvn verify.

I don't think we need to test against a specific Redis server version.

@videnkz videnkz requested a review from felixbarny November 2, 2021 15:53
@videnkz
Copy link
Contributor Author

videnkz commented Nov 2, 2021

Please rename the tests extending AbstractRedisInstrumentationTest so that they end with IT (integration test). This makes them not run when doing a mvn test, only a mvn verify.

I don't think we need to test against a specific Redis server version.

Done

@felixbarny
Copy link
Member

/test

@felixbarny
Copy link
Member

run elasticsearch-ci/docs

@felixbarny felixbarny merged commit e8e4876 into elastic:master Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-java community Issues and PRs created by the community triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redis fails to start sometimes in CI
4 participants