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

test: add go test cases for txn-context-enabled config #2530

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

PokIsemaine
Copy link
Contributor

@PokIsemaine PokIsemaine commented Sep 9, 2024

fix: #2514

I added a config test of txn-context-enabled to the not particularly time-consuming go test cases

Also, I've noticed that some of the tests have resp3-enabled related tests, which require multiple layers of nesting and branching if combined with txn-context-enabled.

To alleviate this, I tried adding util. GenerateConfigsMatrix to generate possible config combinations, which you learn about by using TestGenerateConfigsMatrix in config_test.go. At the moment, only YesNo configs are supported, but I think this will provide some convenience for config-related testing, and we can support more types later if needed

@PokIsemaine PokIsemaine changed the title test: add go test cases for txn_context_enabled config test: add go test cases for txn-context-enabled config Sep 9, 2024
@PokIsemaine PokIsemaine changed the title test: add go test cases for txn-context-enabled config test: add go test cases for txn_context_enabled config Sep 9, 2024
@PokIsemaine PokIsemaine changed the title test: add go test cases for txn_context_enabled config test: add go test cases for txn-context-enabled config Sep 9, 2024
}

// / GenerateConfigsMatrix generates all possible combinations of config options
func GenerateConfigsMatrix(configOptions []ConfigOptions) ([]KvrocksServerConfigs, error) {
Copy link
Member

Choose a reason for hiding this comment

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

So this only return false when options is error?

Comment on lines +63 to +66
currentConfigBackup := make(KvrocksServerConfigs, len(currentConfig))
for k, v := range currentConfig {
currentConfigBackup[k] = v
}
Copy link
Member

Choose a reason for hiding this comment

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

Copying this might be heavy weight but lgtm since it's just testing...

@mapleFU
Copy link
Member

mapleFU commented Sep 9, 2024

Retrigger CI, would merge if all ci passed

Copy link

sonarcloud bot commented Sep 9, 2024

@git-hulk git-hulk merged commit 9fc8ffa into apache:unstable Sep 10, 2024
36 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add go test cases for txn_context_enabled config
3 participants