Skip to content

Adding manual tests list #1446

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,4 @@ custom-gcl.hash
!NOTICE.txt

!internal/fourslash/_scripts/failingTests.txt
!internal/fourslash/_scripts/manualTests.txt
17 changes: 13 additions & 4 deletions internal/fourslash/_scripts/convertFourslash.mts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const stradaFourslashPath = path.resolve(import.meta.dirname, "../", "../", "../
let inputFileSet: Set<string> | undefined;

const failingTestsPath = path.join(import.meta.dirname, "failingTests.txt");
const manualTestsPath = path.join(import.meta.dirname, "manualTests.txt");
const helperFilePath = path.join(import.meta.dirname, "../", "tests", "util_test.go");

const outputDir = path.join(import.meta.dirname, "../", "tests", "gen");
Expand All @@ -21,6 +22,14 @@ function getFailingTests(): Set<string> {
return new Set(failingTestsList);
}

function getManualTests(): Set<string> {
if (!fs.existsSync(manualTestsPath)) {
return new Set();
}
const manualTestsList = fs.readFileSync(manualTestsPath, "utf-8").split("\n").map(line => line.trim()).filter(line => line.length > 0);
return new Set(manualTestsList);
}

export function main() {
const args = process.argv.slice(2);
const inputFilesPath = args[0];
Expand All @@ -36,13 +45,13 @@ export function main() {
fs.mkdirSync(outputDir, { recursive: true });

generateHelperFile();
parseTypeScriptFiles(getFailingTests(), stradaFourslashPath);
parseTypeScriptFiles(getFailingTests(), getManualTests(), stradaFourslashPath);
console.log(unparsedFiles.join("\n"));
const gofmt = which.sync("go");
cp.execFileSync(gofmt, ["tool", "mvdan.cc/gofumpt", "-lang=go1.24", "-w", outputDir]);
}

function parseTypeScriptFiles(failingTests: Set<string>, folder: string): void {
function parseTypeScriptFiles(failingTests: Set<string>, manualTests: Set<string>, folder: string): void {
const files = fs.readdirSync(folder);

files.forEach(file => {
Expand All @@ -53,9 +62,9 @@ function parseTypeScriptFiles(failingTests: Set<string>, folder: string): void {
}

if (stat.isDirectory()) {
parseTypeScriptFiles(failingTests, filePath);
parseTypeScriptFiles(failingTests, manualTests, filePath);
}
else if (file.endsWith(".ts")) {
else if (file.endsWith(".ts") && !manualTests.has(file.slice(0, -3))) {
const content = fs.readFileSync(filePath, "utf-8");
const test = parseFileContent(file, content);
if (test) {
Expand Down
1 change: 0 additions & 1 deletion internal/fourslash/_scripts/failingTests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ TestCompletionListFunctionMembers
TestCompletionListInArrowFunctionInUnclosedCallSite01
TestCompletionListInClassExpressionWithTypeParameter
TestCompletionListInClassStaticBlocks
TestCompletionListInClosedFunction05
TestCompletionListInComments
TestCompletionListInExtendsClause
TestCompletionListInImportClause01
Expand Down
52 changes: 52 additions & 0 deletions internal/fourslash/_scripts/makeManual.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import * as fs from "fs";
import * as path from "path";

const scriptsDir = import.meta.dirname;
const manualTestsPath = path.join(scriptsDir, "manualTests.txt");
const genDir = path.join(scriptsDir, "../", "tests", "gen");
const manualDir = path.join(scriptsDir, "../", "tests", "manual");

function main() {
const args = process.argv.slice(2);

if (args.length === 0) {
console.error("Please provide the name of the generated test file.");
process.exit(1);
}

const testName = args[0];
const testFileName = testName;
const genTestFile = path.join(genDir, testFileName + "_test.go");
if (!fs.existsSync(genTestFile)) {
console.error(`Test file not found: '${genTestFile}'. Make sure the test exists in the gen directory first.`);
process.exit(1);
}

if (!fs.existsSync(manualDir)) {
fs.mkdirSync(manualDir, { recursive: true });
}

const manualTestFile = path.join(manualDir, path.basename(genTestFile));
renameAndRemoveSkip(genTestFile, manualTestFile);

let manualTests: string[] = [];
if (fs.existsSync(manualTestsPath)) {
const content = fs.readFileSync(manualTestsPath, "utf-8");
manualTests = content.split("\n").map(line => line.trim()).filter(line => line.length > 0);
}

if (!manualTests.includes(testName)) {
manualTests.push(testName);
manualTests.sort();
fs.writeFileSync(manualTestsPath, [...manualTests, ""].join("\n"), "utf-8");
}
}

function renameAndRemoveSkip(genFilePath: string, manualFilePath: string) {
const content = fs.readFileSync(genFilePath, "utf-8");
const updatedContent = content.replace(/^\s*t\.Skip\(\)\s*$/m, "");
fs.writeFileSync(manualFilePath, updatedContent, "utf-8");
fs.rmSync(genFilePath);
}

main();
1 change: 1 addition & 0 deletions internal/fourslash/_scripts/manualTests.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
completionListInClosedFunction05
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

func TestCompletionListInClosedFunction05(t *testing.T) {
t.Parallel()
t.Skip()

defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `function foo(x: string, y: number, z: boolean) {
function bar(a: number, b: string = "hello", c: typeof x = "hello") {
Expand All @@ -20,7 +20,7 @@ func TestCompletionListInClosedFunction05(t *testing.T) {
f.VerifyCompletions(t, "1", &fourslash.CompletionsExpectedList{
IsIncomplete: false,
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
CommitCharacters: &[]string{},
CommitCharacters: &defaultCommitCharacters,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a silly question but why can't we generate this one automatically?

Copy link
Member

Choose a reason for hiding this comment

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

There's no simplified way to compute what the commit characters should be other than the actual implementation. The porting script currently sets empty commit characters if isNewIdentifierLocation is true, which is an approximation that does not always hold, like in this test. An alternative would be to ignore commit characters for all ported tests, but I didn't want to do that and end up with no coverage of it.

Copy link
Member

Choose a reason for hiding this comment

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

Would it be simpler to just keep a list of these in the generator script?

Copy link
Member

Choose a reason for hiding this comment

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

For this case, probably, but there are other tests that need other types of manual fixes, and tests that will need to be at least partially manually ported because they have things like for loops and functions.

EditRange: ignored,
},
Items: &fourslash.CompletionsExpectedItems{
Expand Down
Loading
Loading