Skip to content

Accurately recognize fourslash test as submodule #2068

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 12, 2025
Merged

Accurately recognize fourslash test as submodule #2068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f9346a
don't diff non-submodule test
gabritto Nov 12, 2025
9957993
Merge branch 'main' into gabritto/nodiff
gabritto Nov 12, 2025
3b01752
update baselines
gabritto Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion internal/fourslash/baselineutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,13 @@ func getBaselineExtension(command string) string {
}
}

func getBaselineOptions(command string) baseline.Options {
func getBaselineOptions(command string, testPath string) baseline.Options {
subfolder := "fourslash/" + normalizeCommandName(command)
if !isSubmoduleTest(testPath) {
return baseline.Options{
Subfolder: subfolder,
}
}
switch command {
case "Smart Selection":
return baseline.Options{
Expand Down Expand Up @@ -229,6 +234,10 @@ func getBaselineOptions(command string) baseline.Options {
}
}

func isSubmoduleTest(testPath string) bool {
return strings.Contains(testPath, "fourslash/tests/gen") || strings.Contains(testPath, "fourslash/tests/manual")
Copy link
Member

@jakebailey jakebailey Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about this, but runtime.Callers always returns fourward slash paths, so it should work.

Copy link
Member Author

@gabritto gabritto Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hadn't really thought about that, but I tested on Windows. 🙂

}
Comment on lines +237 to +239
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path matching using strings.Contains could produce false positives if 'fourslash/tests/gen' or 'fourslash/tests/manual' appears elsewhere in the absolute path (e.g., in a user's directory name). Consider using filepath.Base(filepath.Dir(testPath)) or a more robust path matching approach like checking if the path ends with these patterns or using path separators for more precise matching.

Copilot uses AI. Check for mistakes.

func normalizeCommandName(command string) string {
words := strings.Fields(command)
command = strings.Join(words, "")
Expand Down
8 changes: 5 additions & 3 deletions internal/fourslash/fourslash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io"
"maps"
"runtime"
"slices"
"strings"
"testing"
Expand Down Expand Up @@ -203,9 +204,10 @@ func NewFourslash(t *testing.T, capabilities *lsproto.ClientCapabilities, conten
}
f.activeFilename = f.testData.Files[0].fileName

_, testPath, _, _ := runtime.Caller(1)
t.Cleanup(func() {
inputWriter.Close()
f.verifyBaselines(t)
f.verifyBaselines(t, testPath)
})
return f
}
Expand Down Expand Up @@ -2243,9 +2245,9 @@ func (f *FourslashTest) getRangeText(r *RangeMarker) string {
return script.content[r.Range.Pos():r.Range.End()]
}

func (f *FourslashTest) verifyBaselines(t *testing.T) {
func (f *FourslashTest) verifyBaselines(t *testing.T, testPath string) {
for command, content := range f.baselines {
baseline.Run(t, getBaselineFileName(t, command), content.String(), getBaselineOptions(command))
baseline.Run(t, getBaselineFileName(t, command), content.String(), getBaselineOptions(command, testPath))
}
}

Expand Down
0 ...ayHints/inlayHintsTupleTypeCrash.baseline → ...ayHints/inlayHintsTupleTypeCrash.baseline
View file
Open in desktop
File renamed without changes.
24 changes: 0 additions & 24 deletions ...baselines/reference/submodule/fourslash/inlayHints/inlayHintsTupleTypeCrash.baseline.diff
View file
Open in desktop

This file was deleted.

Loading