Add -check-skipped flag to identify passing todo tests

parser/CLAUDE.md

@@ -9,3 +9,13 @@ go test ./parser/... -timeout 5s
 ```
 
 The tests are very fast. If a test is timing out, it indicates a bug (likely an infinite loop in the parser).
+
+## Checking Skipped Tests
+
+After fixing parser issues, check if any skipped tests now pass:
+
+```bash
+go test ./parser -check-skipped -v 2>&1 | grep "PASSES NOW"
+```
+
+Tests that output `PASSES NOW` can have their `todo` flag removed from `metadata.json`. This helps identify when parser improvements fix multiple tests at once.

parser/parser_test.go

@@ -3,6 +3,7 @@ package parser_test
 import (
 	"context"
 	"encoding/json"
+	"flag"
 	"os"
 	"path/filepath"
 	"strings"
@@ -12,6 +13,10 @@ import (
 	"github.com/kyleconroy/doubleclick/parser"
 )
 
+// checkSkipped runs skipped todo tests to see which ones now pass.
+// Use with: go test ./parser -check-skipped -v
+var checkSkipped = flag.Bool("check-skipped", false, "Run skipped todo tests to see which ones now pass")
+
 // testMetadata holds optional metadata for a test case
 type testMetadata struct {
 	Todo       bool   `json:"todo,omitempty"`
@@ -87,15 +92,18 @@ func TestParser(t *testing.T) {
 				}
 			}
 
-			// Skip tests marked with skip: true
+			// Skip tests marked with skip: true (these cause infinite loops or other critical issues)
 			if metadata.Skip {
 				t.Skip("Skipping: skip is true in metadata")
 			}
 
 			// Skip tests where explain is explicitly false (e.g., ClickHouse couldn't parse it)
+			// Unless -check-skipped is set and the test is a todo test
 			if metadata.Explain != nil && !*metadata.Explain {
-				t.Skipf("Skipping: explain is false in metadata")
-				return
+				if !(*checkSkipped && metadata.Todo) {
+					t.Skipf("Skipping: explain is false in metadata")
+					return
+				}
 			}
 
 			// Parse the query
@@ -107,7 +115,11 @@ func TestParser(t *testing.T) {
 					return
 				}
 				if metadata.Todo {
-					t.Skipf("TODO: Parser does not yet support: %s (error: %v)", query, err)
+					if *checkSkipped {
+						t.Skipf("STILL FAILING (parse error): %v", err)
+					} else {
+						t.Skipf("TODO: Parser does not yet support: %s (error: %v)", query, err)
+					}
 					return
 				}
 				t.Fatalf("Parse error: %v\nQuery: %s", err, query)
@@ -122,7 +134,11 @@ func TestParser(t *testing.T) {
 
 			if len(stmts) == 0 {
 				if metadata.Todo {
-					t.Skipf("TODO: Parser returned no statements for: %s", query)
+					if *checkSkipped {
+						t.Skipf("STILL FAILING (no statements): parser returned no statements")
+					} else {
+						t.Skipf("TODO: Parser returned no statements for: %s", query)
+					}
 					return
 				}
 				t.Fatalf("Expected at least 1 statement, got 0\nQuery: %s", query)
@@ -132,7 +148,11 @@ func TestParser(t *testing.T) {
 			_, jsonErr := json.Marshal(stmts[0])
 			if jsonErr != nil {
 				if metadata.Todo {
-					t.Skipf("TODO: JSON serialization failed: %v", jsonErr)
+					if *checkSkipped {
+						t.Skipf("STILL FAILING (JSON serialization): %v", jsonErr)
+					} else {
+						t.Skipf("TODO: JSON serialization failed: %v", jsonErr)
+					}
 					return
 				}
 				t.Fatalf("JSON marshal error: %v\nQuery: %s", jsonErr, query)
@@ -150,7 +170,11 @@ func TestParser(t *testing.T) {
 				actual := strings.TrimSpace(parser.Explain(stmts[0]))
 				if actual != expected {
 					if metadata.Todo {
-						t.Skipf("TODO: Explain output mismatch\nQuery: %s\nExpected:\n%s\n\nGot:\n%s", query, expected, actual)
+						if *checkSkipped {
+							t.Skipf("STILL FAILING (explain mismatch):\nExpected:\n%s\n\nGot:\n%s", expected, actual)
+						} else {
+							t.Skipf("TODO: Explain output mismatch\nQuery: %s\nExpected:\n%s\n\nGot:\n%s", query, expected, actual)
+						}
 						return
 					}
 					t.Errorf("Explain output mismatch\nQuery: %s\nExpected:\n%s\n\nGot:\n%s", query, expected, actual)
@@ -165,12 +189,21 @@ func TestParser(t *testing.T) {
 				actualAST := strings.TrimSpace(string(actualASTBytes))
 				if actualAST != expectedAST {
 					if metadata.Todo {
-						t.Skipf("TODO: AST JSON mismatch\nQuery: %s\nExpected:\n%s\n\nGot:\n%s", query, expectedAST, actualAST)
+						if *checkSkipped {
+							t.Skipf("STILL FAILING (AST mismatch):\nExpected:\n%s\n\nGot:\n%s", expectedAST, actualAST)
+						} else {
+							t.Skipf("TODO: AST JSON mismatch\nQuery: %s\nExpected:\n%s\n\nGot:\n%s", query, expectedAST, actualAST)
+						}
 						return
 					}
 					t.Errorf("AST JSON mismatch\nQuery: %s\nExpected:\n%s\n\nGot:\n%s", query, expectedAST, actualAST)
 				}
 			}
+
+			// If we get here with a todo test and -check-skipped is set, the test passes!
+			if metadata.Todo && *checkSkipped {
+				t.Logf("PASSES NOW - can remove todo flag from: %s", entry.Name())
+			}
 		})
 	}
 }

parser/testdata/00053_all_inner_join/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00054_join_string/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00055_join_two_numbers/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00056_join_number_string/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00067_union_all/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00072_in_types/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00076_system_columns_bytes/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00099_join_many_blocks_segfault/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00139_like/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00169_contingency/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00169_join_constant_keys/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00173_group_by_use_nulls/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00178_function_replicate/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00189_time_zones_long/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00205_emptyscalar_subquery_type_mismatch_bug/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00211_shard_query_formatting_aliases/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00216_bit_test_function_family/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00232_format_readable_decimal_size/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00232_format_readable_size/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00250_tuple_comparison/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00264_uniq_many_args/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00283_column_cut/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00291_array_reduce/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00296_url_parameters/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00328_long_case_construction/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00364_java_style_denormals/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00367_visible_width_of_array_tuple_enum/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00381_first_significant_subdomain/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00431_if_nulls/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00434_tonullable/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00436_convert_charset/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00436_fixed_string_16_comparisons/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00518_extract_all_and_empty_matches/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00541_to_start_of_fifteen_minutes/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00552_logical_functions_simple/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00552_logical_functions_ternary/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00552_logical_functions_uint8_as_bool/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00552_or_nullable/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00591_columns_removal_union_all/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00644_different_expressions_with_same_alias/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00667_compare_arrays_of_different_types/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00757_enum_defaults_const/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00759_kodieg/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00801_daylight_saving_time_hour_underflow/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00940_order_by_read_in_order_query_plan/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00973_uniq_non_associativity/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00974_full_outer_join/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00975_move_partition_merge_tree/metadata.json

@@ -1 +1,3 @@
-{"todo": true, "parse_error": true}
+{
+  "parse_error": true
+}

parser/testdata/00977_int_div/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00978_ml_math/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}

parser/testdata/00997_trim/metadata.json

@@ -1 +1,3 @@
-{"explain":false,"todo": true}
+{
+  "explain": false
+}