Audit how `t.Skip()` is being used in tests

[angelamayxie]

It was discovered in dolthub/go-mysql-server#3607 that the majority of tests in TestQueryPlans (test cases in PlanTests) were getting skipped because t.Skip was being called in the incorrect run. This has since been fixed, but we need to make sure that in our other tests, t.Skip() is actually being called in the correct place to avoid skipping tests that aren't supposed to be skipped.

[meethereum]

Hi! I’m interested in working on this issue. I took a look at the current code, and from what I can tell the skip handling is scoped to the per-query subtest on this line , so a single t.Skip() should only affect that case rather than cause later cases to be skipped.

Thats being said, I may be misunderstanding Go’s subtest semantics, though. Could you share how you determined that the tests were being skipped, since I was unable to figure out the same?

[angelamayxie]

@meethereum That line is correct behavior. I had written it in dolthub/go-mysql-server#3607 to fix the skipping issue. The tests in TestQueryPlans should not be skipped anymore. I had discovered the tests were being skipped because tests that should've failed were not failing.

The purpose of this bug is to audit how t.Skip() is used in our other tests and to make sure it's being used correctly.

I've updated the original issue description to be more clear.

[meethereum]

Makes sense
I will see what I can do
Thank you @angelamayxie

Update : I looked for the same patterns rigorously, could not really find any other tests making the same mistake.
I will try picking some other issue maybe