Skip to content

Refactor TestingMySqlServer constructor to take version numbers#11105

Closed
grantatspothero wants to merge 1 commit intotrinodb:masterfrom
grantatspothero:gn/extendMysqlTestContainerWithVersion
Closed

Refactor TestingMySqlServer constructor to take version numbers#11105
grantatspothero wants to merge 1 commit intotrinodb:masterfrom
grantatspothero:gn/extendMysqlTestContainerWithVersion

Conversation

@grantatspothero
Copy link
Copy Markdown
Contributor

Description

Allows version-based testing of mysql. For example, testing specific features from mysql8 that are not supported in mysql5.

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

}

public TestingMySqlServer(String dockerImageName, boolean globalTransactionEnable)
public TestingMySqlServer(int majorVersion, int minorVersion, int patchVersion, boolean globalTransactionEnable)
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.

If the refactoring purpose is

Allows version-based testing of mysql. For example, testing specific features from mysql8 that are not supported in mysql5.

the original code should be sufficient.

Comment on lines +36 to +38
public final int majorVersion;
public final int minorVersion;
public final int patchVersion;
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.

What about adding public static constants for the versions to test - LATEST_IMAGE, DEFAULT_IMAGE?
That should be sufficient?

@grantatspothero
Copy link
Copy Markdown
Contributor Author

Closing due to this PR:
#11106

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

Development

Successfully merging this pull request may close these issues.

3 participants