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

Adding cloud tests #1575

Merged
merged 47 commits into from
Jul 31, 2024
Merged

Adding cloud tests #1575

merged 47 commits into from
Jul 31, 2024

Conversation

mzitnik
Copy link
Contributor

@mzitnik mzitnik commented Mar 24, 2024 •
edited by mshustov
Loading

Summary

Adding cloud tests coverage.
closes #1554

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@mzitnik mzitnik marked this pull request as draft March 24, 2024 10:00
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 27, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@chernser
Copy link
Contributor

I think, this change would be good to adjust for running tests against any host not only cloud.
Current version assumes that cloud using https protocol but test host may be another cases. At least correcting this thing would be a great addition.
As for adding to CI - I think we create a separate workflow. Then we may merge it back. We may not require to run tests against cloud, I think.

mzitnik and others added 19 commits July 24, 2024 11:22
chernser
chernser reviewed Jul 30, 2024
View reviewed changes
...house-http-client/src/test/java/com/clickhouse/client/http/ApacheHttpConnectionImplTest.java
@@ -92,6 +88,7 @@ public void testConnection() throws Exception {
}

@Test(groups = { "integration" })
@Ignore("Need to disable the option to provide custom socket factory. note: need to remove it")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we need to remove it?
It is a bit strange to provide custom socket factory, but it was added for some reason. Certificates?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a @mzitnik question from before 😄

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jul 31, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
19.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@Paultagoras Paultagoras merged commit 48d5937 into main Jul 31, 2024
63 of 64 checks passed
@Paultagoras Paultagoras deleted the add-cloud-test-coverage branch July 31, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chernser chernser chernser approved these changes

@Paultagoras Paultagoras Awaiting requested review from Paultagoras

@mshustov mshustov Awaiting requested review from mshustov

Assignees

@Paultagoras Paultagoras

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Run tests against a SMT Cloud instance
5 participants
@mzitnik @CLAassistant @chernser @mshustov @Paultagoras