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

Sort test suites and enable missed tests #1530

Merged
merged 18 commits into from Jul 17, 2019

Conversation

@sehrope
Copy link
Contributor

commented Jul 17, 2019

The first bunch of commits of this PR are to clean up test suite related diffs down the road. I think it'd be good to stick to a style with trailing commas and sorted names of the test classes. Should simplify merging PRs as any conflicts in suite changes would be minimal additions. If anybody knows a way to enforce this selectively via checkstyle that'd be a great addition.

The second to last commit adds some test classes that were not part of a test suite to their nearest suite. I'm not sure how much value the new tests actually provide but I figure either we add them to a suite or remove them from the code base.

The last commit skips over one of those on ancient v8.2 server as it uses uuid data types.

@sehrope

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2019

Looks like the reordering of the tests did cause breakages with some of the timestamp tests. I bet some other test is modifying the client's timezone somewhere and reusing the connection.

@AppVeyorBot

This comment has been minimized.

Copy link

commented Jul 17, 2019

@AppVeyorBot

This comment has been minimized.

Copy link

commented Jul 17, 2019

@sehrope

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2019

Yup that was the issue. ParameterStatusTest modifies the JVM default time zone but does not reset it. Added a commit to fix that and looks like things are passing now. Only reason this wasn't broken before was that it was added as the last test so nothing ran after it completed.

@davecramer

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

Cool, thanks, once this passes I'll push it

sehrope added 2 commits Jul 17, 2019
test: Add server min version check to CompositeTest
Adds a check that the server version is at least 8.3 to CompositeTest
as it uses the uuid data type.

@sehrope sehrope force-pushed the sehrope:enable-missed-tests branch from 0271503 to fc895ba Jul 17, 2019

@AppVeyorBot

This comment has been minimized.

Copy link

commented Jul 17, 2019

@AppVeyorBot

This comment has been minimized.

Copy link

commented Jul 17, 2019

@sehrope

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2019

@davecramer Looks good now and all tests on Travis are passing.

FYI, had to push one more commit to disable one of the newly added tests in the simple query mode test environment. Looks like that combo is not supported (complex types come back as strings).

@davecramer davecramer merged commit aa8778d into pgjdbc:master Jul 17, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.