Skip to content

test: simplify TestUtil.openDB, add tests with various assumeMinServerVersion values#3614

Merged
vlsi merged 1 commit intopgjdbc:masterfrom
vlsi:assume_min_version
May 3, 2025
Merged

test: simplify TestUtil.openDB, add tests with various assumeMinServerVersion values#3614
vlsi merged 1 commit intopgjdbc:masterfrom
vlsi:assume_min_version

Conversation

@vlsi
Copy link
Member

@vlsi vlsi commented May 1, 2025

Previously, the code considered only a few of PGProperty names when creating a database connection. So there was no possibility to set the rest of the properties by passing them with build.properties or -D...

After the change, the code uses all the matching properties from System.properties, so we can increase test coverage.

For instance, the change adds tests with assumeMinServerVersion setting.

I still do not like how Properties contents is copied all over the place, however, we can't do better as we do not have Properties(List<Properties>) factory yet.

This test would have caught #3508 earlier.

The idea behind the change is:

  • Prefer pgjdbc property names (the ones in PGProperty). For instance, previously, build.properties included a property named username which was handled in a special way since PGProperty.USER has a value of user.
    That is the reason I changed server to test.url.PGHOST which effectively means "use it as a default PGHOST value, and pass it via the URL rather than props object)
  • host/port/database was handled separately when building the URL. Now the logic is that any property can be passed to the URL when building the URL with TestUtil. The test.url. prefix allows to configure which properties should be passed with the URL (TestUtil strips the prefix and adds the rest as a query parameter, etc)

@vlsi vlsi force-pushed the assume_min_version branch 14 times, most recently from 10096bb to 4c3dfa4 Compare May 2, 2025 17:45
…rVersion values

Previously, the code considered only a few of PGProperty names when creating a database connection.
So there was no possibility to set the rest of the properties by passing them with build.properties or -D...

After the change, the code uses all the matching properties from System.properties,
so we can increase test coverage.

For instance, the change adds tests with assumeMinServerVersion setting.
@vlsi vlsi force-pushed the assume_min_version branch from 4c3dfa4 to 0884ee3 Compare May 2, 2025 19:12
@vlsi vlsi merged commit 9fb6cc1 into pgjdbc:master May 3, 2025
18 checks passed
@vlsi vlsi deleted the assume_min_version branch May 16, 2025 12:44
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.

1 participant

Comments