Skip to content

Support database aliases in R2DBC - #2599

Draft
bsideup wants to merge 4 commits into
mainfrom
db_aliases
Draft

Support database aliases in R2DBC#2599
bsideup wants to merge 4 commits into
mainfrom
db_aliases

Conversation

@bsideup

@bsideup bsideup commented Apr 21, 2020

Copy link
Copy Markdown
Member

No description provided.

bsideup added 3 commits April 17, 2020 10:57
# Conflicts:
#	modules/r2dbc/src/testFixtures/java/org/testcontainers/r2dbc/AbstractR2DBCDatabaseContainerTest.java
@stale

stale Bot commented Jul 20, 2020

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

@stale stale Bot added the stale label Jul 20, 2020
@stale stale Bot removed the stale label Jul 20, 2020

private static final String ACCEPTANCE_FILE_NAME = "container-license-acceptance.txt";

static final Set<String> ACCEPTED_IMAGE_NAMES = new HashSet<>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
static final Set<String> ACCEPTED_IMAGE_NAMES = new HashSet<>();
final Set<String> ACCEPTED_IMAGE_NAMES = new HashSet<>();

Since we use @UtilityClass, all static modifiers in the class can be omitted.

import org.junit.runner.Description;
import org.junit.runners.model.Statement;

public class MockLicenseAcceptanceRule implements TestRule {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO this is not a mock, but more a way to programmatically accept licences, maybe LicenseAcceptanceRule would fit better (just dropping the mock)?

Also renaming MockTestcontainersConfigurationRule to something like TestcontainersConfigurationRolebackRule accordingly would make sense? (stumbled upon this during review and took a bit to understand the idea behind the class)

}
return future.thenApply(it -> {
return ConnectionFactories.find(
return ConnectionFactories.get(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could be replaced by a lambda expression.

@kiview

kiview commented Jul 24, 2020

Copy link
Copy Markdown
Member

Some proposed refactorings submitted in #3019

@kiview

kiview commented Aug 7, 2020

Copy link
Copy Markdown
Member

This branch fails on TestcontainersR2DBCConnectionFactoryTest.failsOnUnknownProvider() (because it does not fail, what irony! 😭 ).

Properties properties = TestcontainersConfiguration.getInstance().getProperties();
return properties.getProperty(
driverPropertyName(options.getRequiredValue(ConnectionFactoryOptions.DRIVER)),
originalDriver

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
originalDriver
options.getRequiredValue(ConnectionFactoryOptions.DRIVER)

This will fix failing TestcontainersR2DBCConnectionFactoryTest.failsOnUnknownProvider() .

Fix is already included in #3019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants