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.

Sign up for GitHub

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

Jump to bottom

[#3209]test(web): doris web e2e test #3230

Merged
merged 19 commits into from May 14, 2024
Merged

[#3209]test(web): doris web e2e test #3230

merged 19 commits into from May 14, 2024

Conversation

LauraXia123
Copy link
Contributor

What changes were proposed in this pull request?

Add test create doris catalog to the web e2e test

Why are the changes needed?

Fix: #3209

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

manual

@LauraXia123 LauraXia123 self-assigned this Apr 30, 2024
@yuqi1129 yuqi1129 closed this Apr 30, 2024
@yuqi1129 yuqi1129 reopened this Apr 30, 2024
@yuqi1129 yuqi1129 closed this Apr 30, 2024
@yuqi1129 yuqi1129 reopened this Apr 30, 2024
@yuqi1129
Copy link
Contributor

@LauraXia123
I have solved the problem related to Doris, there still some issues about fornt-end test, please take a look.

@LauraXia123
Copy link
Contributor Author

@yuqi1129 Thanks a lot. I'll check the left issues.

@LauraXia123 LauraXia123 force-pushed the issue-3209 branch 6 times, most recently from 8f76ac2 to d099f65 Compare May 6, 2024 05:23
@LauraXia123
Copy link
Contributor Author

All the test case have passed, but I can't start the doris container locally, can you try it locally for me? @xunliu

@zhoukangcn
Copy link
Contributor

zhoukangcn commented May 6, 2024 •
edited

All the test case have passed, but I can't start the doris container locally, can you try it locally for me? @xunliu

@LauraXia123 Did you start doris successfully? If you still have problem, I can help

@LauraXia123
Copy link
Contributor Author

image image @zhoukangcn You can take a look my stack information above.

@zhoukangcn
Copy link
Contributor

@LauraXia123 you can exec command docker images|grep doris to make sure doris Image exist (datastrato/gravitino-ci-doris:0.1.3)

If Doris Image not exist, you can use command docker pull datastrato/gravitino-ci-doris:0.1.3 before CI

If Doris Image exist, you can see log in catalogs/catalog-jdbc-doris/build , include testcontainers.log, catalog-jdbc-doris-integration-test.log and send it to me

@LauraXia123
Copy link
Contributor Author

The image is exist but not log files.
image
image

yuqi1129
yuqi1129 reviewed May 13, 2024
View reviewed changes
...st/src/test/java/com/datastrato/gravitino/integration/test/web/ui/CatalogsPageDorisTest.java Outdated
private static GravitinoMetalake metalake;

protected static String gravitinoUri = "http://127.0.0.1:8090";
protected static String dorisUri = "http://127.0.0.1:9030";
Copy link
Contributor

@yuqi1129 yuqi1129 May 13, 2024 •
edited

Choose a reason for hiding this comment

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

Better change dorisUri to dorisJdbcConnectionUri and remove the value http://127.0.0.1:9030 as it's a wrong value, the format should be 'jdbc:mysql:host:port/xxx'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

...st/src/test/java/com/datastrato/gravitino/integration/test/web/ui/CatalogsPageDorisTest.java Outdated Show resolved Hide resolved
@yuqi1129
Copy link
Contributor

Please move on for this PR, thanks.

yuqi1129
yuqi1129 reviewed May 13, 2024
View reviewed changes
build.gradle.kts Outdated
@@ -154,6 +154,7 @@ allprojects {
param.environment("HADOOP_USER_NAME", "datastrato")
param.environment("HADOOP_HOME", "/tmp")
param.environment("PROJECT_VERSION", project.version)
// param.environment("DISPLAY_WEBPAGE_IN_TESTING", true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove DISCLAIMER WEBPAGEIN TESTING directly if it is no longer in use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@yuqi1129
Copy link
Contributor

@xunliu
Please help to take a review if you have time, thanks.

@LauraXia123 LauraXia123 requested a review from xunliu May 13, 2024 07:25
xunliu
xunliu approved these changes May 13, 2024
View reviewed changes
Copy link
Collaborator

@xunliu xunliu left a comment

Choose a reason for hiding this comment

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

When I setting 8GB memory in the Docker, then doris web e2e failed.
When I setting 16GB memory in the Docker, then doris web e2e success.
@LauraXia123 I think you need create a issue to track this problem.

I think we can merged this PR first.

@xunliu
Copy link
Collaborator

xunliu commented May 13, 2024

@LauraXia123
You can setting 16GB memory in the Docker, if CI running in you local success, then we can merge this PR.

@LauraXia123
Copy link
Contributor Author

I can run it success this morning
image

@xunliu
Copy link
Collaborator

xunliu commented May 14, 2024

@LauraXia123 Thank you for your contributions.
LGTM

@xunliu xunliu merged commit 595f377 into datastrato:main May 14, 2024
22 checks passed
@jerryshao
Copy link
Collaborator

Again @xunliu , please follow the steps in MAINTAINERS.md when merging the PR. I really don't want to say this again and again. Merging a PR is very simple, but it will cost time when doing a release without following the steps. You have to a) weigh the necessity to backport; b) add necessary tags.

LauraXia123 added a commit to LauraXia123/gravitino that referenced this pull request May 15, 2024
@LauraXia123 @yuqi1129
[datastrato#3209] test(web): doris web e2e test (datastrato#3230)
a6a3534 
### What changes were proposed in this pull request?

Add test create doris catalog to the web e2e test

### Why are the changes needed?

Fix: datastrato#3209

### Does this PR introduce _any_ user-facing change?
N/A

### How was this patch tested?
manual

---------

Co-authored-by: yuqi <yuqi@datastrato.com>
jerryshao pushed a commit that referenced this pull request May 15, 2024
@LauraXia123 @yuqi1129
[#3209] test(web): doris web e2e test (#3230) (#3398)
b90ee99 
### What changes were proposed in this pull request?

Add test create doris catalog to the web e2e test

### Why are the changes needed?

Fix: #3209

### Does this PR introduce _any_ user-facing change? N/A

### How was this patch tested?
manual

Co-authored-by: yuqi <yuqi@datastrato.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuqi1129 yuqi1129 yuqi1129 left review comments

@xunliu xunliu xunliu approved these changes

Assignees

@LauraXia123 LauraXia123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Subtask] Add test create doris catalog to the web e2e test
5 participants
@LauraXia123 @yuqi1129 @zhoukangcn @xunliu @jerryshao