-
-
Notifications
You must be signed in to change notification settings - Fork 514
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
adding missing test cases for prepared tests #4163
Conversation
…o james/prepareds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some small comments
@@ -822,29 +824,6 @@ var HistorySystemTableScriptTests = []queries.ScriptTest{ | |||
{3, 4}, | |||
}, | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be easier to leave these here with SkipPrepared=true
, but still append to a new list of tests titled BrokenHistorySystemTableScriptTests
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in which case you don't need the comment above
@@ -1073,6 +1113,35 @@ func TestHistorySystemTable(t *testing.T) { | |||
enginetest.TestScript(t, harness, test) | |||
}) | |||
} | |||
for _, test := range BrokenHistorySystemTableScriptTests { | |||
harness.engine = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you verified that the tests that include this cannot reuse the engine?
@@ -66,21 +66,43 @@ func TestQueries(t *testing.T) { | |||
func TestSingleQuery(t *testing.T) { | |||
t.Skip() | |||
|
|||
harness := newDoltHarness(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general it is courteous to limit unnecessary changes that make PRs more difficult to read
Companion PR:
Adds skipped tests for Queries that don't yet work Prepared