Add basic tests for applier by timvaillancourt · Pull Request #1165 · github/gh-ost

Skip to content

Navigation Menu

Sign in

Appearance settings

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up

Appearance settings

Conversation

@timvaillancourt

Copy link Copy Markdown

Collaborator

@timvaillancourt timvaillancourt commented

Aug 14, 2022

edited

Loading

Description

This PR adds basic tests to a few funcs from go/logic/applier.go, also two small fixes were made based on findings in testing

Details:

  • Added tests to func that don't use MySQL in go/logic/applier_test.go
  • Removed extra spaces in sql.BuildDMLUpdateQuery() from go/sql/builder.go
  • Modified applier.generateSqlModeQuery() to avoid duplicate commas:
    • Before example (double-comma):
      sql_mode = CONCAT(@@session.sql_mode, ',,NO_AUTO_VALUE_ON_ZERO')
    • After example:
      sql_mode = CONCAT(@@session.sql_mode, ',NO_AUTO_VALUE_ON_ZERO')

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

@timvaillancourt timvaillancourt requested review from a user and rashiq

August 14, 2022 21:03

@timvaillancourt timvaillancourt merged commit 1fa3d4f into github:master

Sep 6, 2022

@timvaillancourt timvaillancourt deleted the logic-applier-tests branch

September 6, 2022 13:06

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

Reviewers

@rashiq rashiq Awaiting requested review from rashiq rashiq is a code owner

1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

v1.1.6

Development

Successfully merging this pull request may close these issues.

1 participant

@timvaillancourt