DENA-671: refactor: extracted test cases from func & split them into multiple scenarios #26
Conversation
| res := make([]*helper.Issue, len(expected)) | ||
| for i, exp := range expected { | ||
| exp.Rule = rule | ||
| res[i] = exp | ||
| } | ||
| return res |
There was a problem hiding this comment.
nit: this function both mutates an input (via exp.Rule = rule) and returns a separate output. I think it should either be pure, and just return the useful bits, or mutate its input and return nothing
There was a problem hiding this comment.
Riight.. I didn't realise I get a pointer to the issue. Will address separate
rules/msk_topic_config_test.go
Outdated
| var allTests []topicConfigTestCase | ||
| allTests = append(allTests, replicationFactorTests...) | ||
| allTests = append(allTests, compressionTypeTests...) | ||
| allTests = append(allTests, cleanupPolicyTests...) | ||
| allTests = append(allTests, deletePolicyRetentionTimeTests...) | ||
| allTests = append(allTests, deletePolicyTieredStorageTests...) | ||
| allTests = append(allTests, goodConfigTests...) |
There was a problem hiding this comment.
random thought: I really like the iterators in Go 1.23 that would allow us to do this lazily (i.e. loop over all this sub-arrays without constructing a new one)
There was a problem hiding this comment.
Not sure about this one... I think it's clearer what's happening like this.
I think in the implementing Seq func we'd still to iterate through each of these slices, or to unify them and then iterate, just like in here
There was a problem hiding this comment.
Not sure about this one... I think it's clearer what's happening like this. I think in the implementing Seq func we'd still to iterate through each of these slices, or to unify them and then iterate, just like in here
Unless there w as a library for that 😉 https://pkg.go.dev/github.com/matthewhughes934/[email protected]/itertools#ChainSlices (not saying to use it, just something I had some fun playing around with recently)
There was a problem hiding this comment.
:) I'll leave it like this, if you don't mind, as I don't want to introduce a dependency from a repo of some shady dude
* Validate tiered storage not enabled for compacted topics * Validate local retention not defined for compacted topics * Validate retention time not defined for compacted topics
This solves the //nolint:maintidx for the test func & adds more clarity to test groups