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

[#6459]improve(test): Use implicit wait for time-based condition #6460

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zzzk1
Copy link
Contributor

@zzzk1 zzzk1 commented Feb 15, 2025

What changes were proposed in this pull request?

This improvement follows our E2E testing guide by utilizing the wait methods and removing Thread.sleep(sleepTimeMillis).

Why are the changes needed?

Fix: #6459

Does this PR introduce any user-facing change?

no

How was this patch tested?

CI

@tengqm
Copy link
Contributor

tengqm commented Feb 17, 2025

lgtm. Good job.

@jerryshao
Copy link
Contributor

@zzzk1 can you please fix the CI issue here?

@zzzk1
Copy link
Contributor Author

zzzk1 commented Feb 17, 2025

@zzzk1 can you please fix the CI issue here?

Yes, ./gradlew test [--rerun-tasks] -PskipTests -PtestMode=embedded. What should I replace [--rerun-tasks] with if I want to set up E2E tests locally?

@jerryshao
Copy link
Contributor

You can keep --rerun-tasks, don't need to remove or replace with others. You should make sure you have docker environment locally, please check this doc https://gravitino.apache.org/docs/0.8.0-incubating/how-to-test.

@yuqi1129 yuqi1129 closed this Feb 19, 2025
@yuqi1129 yuqi1129 reopened this Feb 19, 2025
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.

[Improvement] Use the implicitly wait feature provided by Selenium to replace Thread.Sleep
4 participants