Tom Clegg wrote:
Don't do the "options ...bool" thing in the test suite. Just add a loadKeepstoresFromConfig bool arg, and add ,true or ,false to the existing runProxy() uses.
OK. I've simplified this further and removed the additional argument to runProxy. I've replaced it with a variable that is set in SetUpSuite for ServerRequiredConfigYmlSuite.
It looks like the additional ServerRequiredConfigYmlSuite ended up being unnecessary, and the new TestGetIndex could be (*ServerRequiredSuite)TestGetIndexUsingConfigYAML()... or is there a subtle reason why the separate suite is needed? (Need to restart servers in between? Need more than 2 keepstores?)
Yeah, the config.yml defines 4 keepstores, so I need them all to be up (or the index test will fail). I've put a comment to that effect in the SetUpSuite for ServerRequiredConfigYmlSuite.
You can avoid the extra "associated revisions" in redmine (which don't go away when you amend/rebase) by tagging commits "16328: blah..." and leaving out the "refs #16328" bit until the merge-to-master commit.
Ah, yes. Will do going forward.
Nit: Even though identifiers in _test.go don't get exported anyway, I'd use private lowercase getIndexWorker instead of GetIndexWorker.
Sure, changed.
The actual code change LGTM.
Thanks, updated changes at a77a856600ef5d22208eff74d909ff6d1dc4664d
Updated by 
Updated by