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

Fix EnterpriseDB database recognition #2767

Merged
merged 2 commits into from
Apr 19, 2022
Merged

Fix EnterpriseDB database recognition #2767

merged 2 commits into from
Apr 19, 2022

Conversation

nvoxland
Copy link
Contributor

@nvoxland nvoxland commented Apr 15, 2022 •
edited
Loading

Description

The result of version() for EDB doesn't have "EnterpriseDB" at the beginning of the string, but can by anywhere in it.

This fixes the check to better handle different cases.

Fixes DAT-9954

Test Harness tests: liquibase/liquibase-test-harness#209

@nvoxland nvoxland mentioned this pull request Apr 15, 2022
Closed
8 tasks
@nvoxland nvoxland changed the title [DAT-9954] Fix EDB database recognition Fix EnterpriseDB database recognition Apr 15, 2022
@nvoxland nvoxland requested a review from suryaaki2 April 15, 2022 18:41
@nvoxland nvoxland mentioned this pull request Apr 15, 2022
Merged
@github-actions
Copy link

github-actions bot commented Apr 15, 2022 •
edited
Loading

Unit Test Results

  4 136 files  +11    4 136 suites  +11   36m 9s ⏱️ - 8m 32s
  4 343 tests  - 89    4 125 ✔️  - 93     218 💤 +4  0 ❌ ±0 
47 113 runs  +33  42 477 ✔️ +29  4 636 💤 +4  0 ❌ ±0 

Results for commit b59e0a1. ± Comparison against base commit 8c7274a.

♻️ This comment has been updated with latest results.

@XDelphiGrl XDelphiGrl self-assigned this Apr 18, 2022
@XDelphiGrl
Copy link
Contributor

XDelphiGrl commented Apr 18, 2022 •
edited
Loading

  • Fix is straightforward.
  • Unit test includes both a positive and negative test. Unit test also validates Liquibase returns the correct database version when the connection string is jdbc:postgres.
  • Fix meets criteria to merge without manual testing.
    • GitHub issues prevented the test harness and functional tests from running. Given that this is a fix that impacts a database platform undergoing initial certification on the test harness, I'm going to wait until those tests pass before approving this merge.

Passing Functional Tests
Passing Test Harness Tests

CC @nvoxland @kataggart @kristyldatical @nvoxland

@kataggart kataggart added this to the NEXT milestone Apr 18, 2022
@nvoxland nvoxland merged commit 3043786 into master Apr 19, 2022
@nvoxland nvoxland deleted the DAT-9954 branch April 19, 2022 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@suryaaki2 suryaaki2 suryaaki2 approved these changes

Assignees

@XDelphiGrl XDelphiGrl

Labels
None yet
Projects
Liquibase Open Source
Archived in project
Milestone
v4.10.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nvoxland @XDelphiGrl @suryaaki2 @kataggart @yodzhubeiskyi