fix: resolve \ir relative paths in single file pg_tap tests#4845
fix: resolve \ir relative paths in single file pg_tap tests#48457ttp wants to merge 6 commits intosupabase:developfrom
Conversation
internal/db/test/test.go
Outdated
| relPath := dockerPath | ||
| if path.Ext(dockerPath) != "" { | ||
| relPath = path.Base(dockerPath) | ||
| } | ||
| cmd = append(cmd, relPath) | ||
| binds[i] = fmt.Sprintf("%s:%s:ro", fp, dockerPath) |
There was a problem hiding this comment.
What if the file imported by \ir imports another file using \ir?
To support this properly, we need to traverse all the files imported. Easiest way is to use a regex expression. You can refer to this example https://github.com/supabase/cli/blob/develop/pkg/function/deno.go#L114
There was a problem hiding this comment.
If you end up implementing that, please also write a unit test to cover these cases.
There was a problem hiding this comment.
Done 💚😅!
Added traverseImports with regex pattern matching and unit tests for nested imports, circular imports and relative paths
📝 WalkthroughWalkthroughAdds recursive discovery of PostgreSQL Changes
Sequence Diagram(s)mermaid CLI->>FS: receive test file paths Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/db/test/test.go`:
- Line 30: The regex in var irPattern currently anchors to line start so
indented \ir directives are missed; update the regular expression in var
irPattern (the compiled pattern used to find `\ir` directives) to allow optional
leading whitespace before the backslash (e.g. add `\s*` after the `^`), keeping
the rest of the capture group intact so filenames are still captured the same
way.
- Around line 34-66: The multi-dir failure is caused by always reducing
dockerPath to path.Base for test files; change the logic in the loop over
allFiles (the block that sets workingDir, computes dockerPath, and appends to
cmd/binds) so that path.Base(dockerPath) is used only when the directory of
dockerPath equals the chosen workingDir (which is set from the first file); for
files from other directories keep the full dockerPath when appending to cmd (so
pg_prove can find them) and still add the corresponding binds entry; update the
code around traverseImports, testFiles, allFiles, cmd, workingDir, and binds to
reflect this conditional basename behavior.
🧹 Nitpick comments (1)
internal/db/test/test_test.go (1)
107-141: Strengthen traversal assertions to validate file names, not just counts.
Right now, Line 116/128/139 only checks length, so a wrong path could still pass. Consider asserting the expected file set.💡 Example tightening for all three subtests
- assert.Len(t, result, 2) + assert.ElementsMatch(t, []string{"main.sql", "helper.sql"}, result) ... - assert.Len(t, result, 3) + assert.ElementsMatch(t, []string{"main.sql", "level1.sql", "level2.sql"}, result) ... - assert.Len(t, result, 2) + assert.ElementsMatch(t, []string{"a.sql", "b.sql"}, result)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/db/test/test.go`:
- Around line 125-155: traverseImports currently uses q as a LIFO stack which
reverses testFiles order; change traversal to FIFO so imports are processed in
input order (preserve order of testFiles in result). Modify the loop in
traverseImports to consume q from the front (e.g., iterate with an index over q
or dequeue from the head) instead of popping the last element, keep the same
logic for marking seen, reading files, and appending discovered imports to q so
the final result order remains stable for Run and pg_prove behavior.
2 participants
Problem
Running
supabase db test <file>fails when the test file uses\ir ./other_file.sqlto include other files.The test runner passes absolute paths to
pg_prove, breaking PostgreSQL's\irrelative path resolution.Solution
Use
path.Base()to pass filenames instead of absolute paths topg_prove, while maintaining the correct working directory.This allows
\ir ./file.sqlto resolve correctly. Directory arguments remain unchanged for backward compatibility.Related
Summary by CodeRabbit