Skip to content

test: add test parse ipv6 - #417

Merged
NguyenHoangSon96 merged 1 commit into
mainfrom
test/add-test-ipv6
Aug 4, 2026
Merged

test: add test parse ipv6#417
NguyenHoangSon96 merged 1 commit into
mainfrom
test/add-test-ipv6

Conversation

@NguyenHoangSon96

@NguyenHoangSon96 NguyenHoangSon96 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Closes #

Proposed Changes

  • Add tests for ipv6, I only check if our clients can parse the addresses correctly. Real communication tests need to be created inside a machine executor in CircleCI, and It is not worth the efforts, already talked with Jakub about this.
  • Add comments to let users know that ipv6 must be used with square brackets. This is because we are using java.net.URI package.

I don't think these changes need to be in CHANGELOG.md

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • Tests pass
  • Commit messages are conventional
  • Sign CLA (if not already signed)

@NguyenHoangSon96 NguyenHoangSon96 self-assigned this Aug 3, 2026
Copilot AI review requested due to automatic review settings August 3, 2026 13:47
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.73%. Comparing base (fd81e36) to head (053c94b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   88.29%   88.73%   +0.43%     
==========================================
  Files          21       21              
  Lines        1547     1553       +6     
  Branches      280      281       +1     
==========================================
+ Hits         1366     1378      +12     
+ Misses         83       77       -6     
  Partials       98       98              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds coverage for IPv6 URL parsing in InfluxDBClient initialization and introduces a validation guard in the FlightSQL client to reject URLs that don’t parse into a host (e.g., unbracketed IPv6 literals).

Changes:

  • Add a JUnit test covering valid/invalid IPv6 URL forms passed to InfluxDBClient.getInstance(...).
  • Add a uri.getHost() == null validation in FlightSqlClient#createLocation to fail fast on invalid URLs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/test/java/com/influxdb/v3/client/InfluxDBClientTest.java Adds IPv6 parsing test cases for InfluxDBClient.getInstance(...).
src/main/java/com/influxdb/v3/client/internal/FlightSqlClient.java Adds host-null guard when converting ClientConfig.host into a gRPC Location.
Suppressed comments (1)

src/test/java/com/influxdb/v3/client/InfluxDBClientTest.java:66

  • InfluxDBClient is AutoCloseable and other tests in this class use try-with-resources. Creating clients here without closing them can leak Netty/Arrow resources across the test suite. Wrap the successful cases in try-with-resources (and also use the record accessor url()).
            } else {
                InfluxDBClient.getInstance(test.url, "my-token".toCharArray(), "bucket0");
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/java/com/influxdb/v3/client/InfluxDBClientTest.java Outdated
Comment thread src/main/java/com/influxdb/v3/client/internal/FlightSqlClient.java Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (11)

src/main/java/com/influxdb/v3/client/InfluxDBClient.java:508

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.
     * @param host        the URL of the InfluxDB server. If IPv6 is using it must be wrapped inside brackets.

src/main/java/com/influxdb/v3/client/InfluxDBClient.java:564

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.
     * @param connectionString connection string. If IPv6 is using it must be wrapped inside brackets.

src/main/java/com/influxdb/v3/client/InfluxDBClient.java:587

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.
     *   <li>INFLUX_HOST - cloud/server URL. If IPv6 is using it must be wrapped inside brackets. <i>required</i></li>

src/main/java/com/influxdb/v3/client/InfluxDBClient.java:600

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.
     *   <li>influx.host - cloud/server URL. If IPv6 is using it must be wrapped inside brackets. <i>required</i></li>

src/main/java/com/influxdb/v3/client/config/ClientConfig.java:498

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.
         * Sets the URL of the InfluxDB server. If IPv6 is using it must be wrapped inside brackets.

src/main/java/com/influxdb/v3/client/config/ClientConfig.java:844

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.
         * @param connectionString connection string in URL format. If IPv6 is using it must be wrapped inside brackets.

src/main/java/com/influxdb/v3/client/config/ClientConfig.java:394

  • ClientConfig.validate() currently rethrows URISyntaxException using e.getMessage() and drops the cause, which makes the error message unstable (depends on JDK wording and may not include "Invalid URL.") and loses the underlying exception chain. Consider normalizing the message (and handling blank host) while preserving the cause for debugging.
        } catch (URISyntaxException e) {
            throw new IllegalArgumentException(e.getMessage());
        }

src/test/java/com/influxdb/v3/client/InfluxDBClientTest.java:68

  • parseIpv6() creates InfluxDBClient instances without closing them. Since InfluxDBClient is AutoCloseable, this can leak resources (threads/connections) across tests. Also, the method declares checked exceptions that it doesn't throw directly.
    @Test
    void parseIpv6() throws UnknownHostException, URISyntaxException {
        record Test(String url, boolean isCorrect) {
        }
        var tests = List.of(

src/test/java/com/influxdb/v3/client/InfluxDBClientTest.java:28

  • These imports are only needed because parseIpv6() declares URISyntaxException/UnknownHostException. If the test no longer declares those checked exceptions, the imports can be removed to avoid unused-import warnings.
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.util.List;

src/main/java/com/influxdb/v3/client/InfluxDBClient.java:486

  • Grammar: "If IPv6 is using it must be wrapped inside brackets" is ungrammatical and a bit unclear. Consider rephrasing for readability.

This issue also appears in the following locations of the same file:

  • line 508
  • line 564
  • line 587
  • line 600
     * @param host     the URL of the InfluxDB server. If IPv6 is using it must be wrapped inside brackets.

src/main/java/com/influxdb/v3/client/config/ClientConfig.java:56

  • The documentation says "hostname or IP address", but the client code uses java.net.URI parsing and requires a full URL (scheme + host). Also, the IPv6 sentence is ungrammatical; consider rewording both for clarity.

This issue also appears in the following locations of the same file:

  • line 498
  • line 844
 *     <li>
 *         <code>host</code> - hostname or IP address of the InfluxDB server.
 *         If IPv6 is using it must be wrapped inside brackets.
 *     </li>

@NguyenHoangSon96
NguyenHoangSon96 force-pushed the test/add-test-ipv6 branch 2 times, most recently from bf918af to 4fe584f Compare August 3, 2026 16:16
InfluxDBClient.getInstance(test.url(), "my-token".toCharArray(), "bucket0")
).hasMessageContaining("Invalid URL.");
} else {
InfluxDBClient.getInstance(test.url(), "my-token".toCharArray(), "bucket0");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Client never closed. In tests it is low priority issue, but existing tests create new client with try-with-resources. It should be used here too, I think.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed, please check again 🙏 .

@alespour alespour left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM now.

@NguyenHoangSon96
NguyenHoangSon96 merged commit 849a4f5 into main Aug 4, 2026
16 checks passed
@NguyenHoangSon96
NguyenHoangSon96 deleted the test/add-test-ipv6 branch August 4, 2026 06:53
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.

3 participants