testscript: add Config.Files #258

rogpeppe · 2024-06-10T21:29:00Z

This makes it possible to pass an arbitrary set of testscript files to be run instead of just a directory, making it possible for the testscript command to pass its command line arguments directly.

In order to check that all the files are actually tested, we need to make the test harness implement independent subtest failure, and it's useful to see the name of the test too so that we can see the name disambiguation logic at work, which makes for changes to some of the other tests too.

Note that the name deduping logic is somewhat improved from similar logic in cmd/testscript, in that it is always guaranteed to produce unique names even in the presence of filenames that look like deduped names.

It's bugged me for a long time that the error messages printed by the `testscript` command do not refer to the actual files passed to the command, but instead to a temporary file created for the purposes of the command. This change alters the testscript command so that it avoids creating an extra copy of the script file, instead using the new ability of the testscript package to interpret explicitly provided files instead. Gratifyingly, this also simplifies the logic quite a bit. Note: this is dependent on #258, so should not be reviewed until that lands.

abhinav

_{(Don't mind me. Just a drive-by review.)}

Excellent! I've been meaning to open an issue to request something like.

abhinav · 2024-06-10T22:04:27Z

testscript/testscript.go

+		for i := 1; ; i++ {
+			if !names[name] {
+				break
+			}


optional: this can be collapsed.

Suggested change

for i := 1; ; i++ {

if !names[name] {

break

}

for i := 1; names[name]; i++ {

abhinav · 2024-06-10T22:09:07Z

testscript/testdata/testscript_explicit_files.txt

+FAIL: $WORK/y/bar.txtar:1: told to exit with code 1
+** RUN bar#1#1 **
+> echoandexit 1 '' 'bar#1#1 failure'
+[stderr]
+bar#1#1 failure
+FAIL: $WORK/y/bar#1.txtar:1: told to exit with code 1


Suggested change

FAIL: $WORK/y/bar.txtar:1: told to exit with code 1

** RUN bar#1#1 **

> echoandexit 1 '' 'bar#1#1 failure'

[stderr]

bar#1#1 failure

FAIL: $WORK/y/bar#1.txtar:1: told to exit with code 1

FAIL: $WORK${/}y${/}bar.txtar:1: told to exit with code 1

** RUN bar#1#1 **

> echoandexit 1 '' 'bar#1#1 failure'

[stderr]

bar#1#1 failure

FAIL: $WORK${/}y${/}bar#1.txtar:1: told to exit with code 1

close, but needs cmpenv. done, thanks!

mvdan · 2024-06-11T08:08:31Z

testscript/testscript.go

@@ -136,6 +137,11 @@ type Params struct {
 	// Dir is interpreted relative to the current test directory.
 	Dir string

+	// Files holds a set of script files. If Dir is empty and this


say filenames to be clearer?

This makes it possible to pass an arbitrary set of testscript files to be run instead of just a directory, making it possible for the testscript command to pass its command line arguments directly. In order to check that all the files are actually tested, we need to make the test harness implement independent subtest failure, and it's useful to see the name of the test too so that we can see the name disambiguation logic at work, which makes for changes to some of the other tests too. Note that the name deduping logic is somewhat improved from similar logic in cmd/testscript, in that it is always guaranteed to produce unique names even in the presence of filenames that look like deduped names.

It's bugged me for a long time that the error messages printed by the `testscript` command do not refer to the actual files passed to the command, but instead to a temporary file created for the purposes of the command. This change alters the testscript command so that it avoids creating an extra copy of the script file, instead using the new ability of the testscript package to interpret explicitly provided files instead. Gratifyingly, this also simplifies the logic quite a bit. Note: this is dependent on #258, so should not be reviewed until that lands.

rogpeppe requested a review from mvdan June 10, 2024 21:29

rogpeppe force-pushed the 047-testscript-config-files branch from b6450f9 to 92199db Compare June 10, 2024 21:52

rogpeppe mentioned this pull request Jun 10, 2024

cmd/testscript: do not create an extra temporary directory #259

Merged

abhinav approved these changes Jun 10, 2024

View reviewed changes

abhinav reviewed Jun 10, 2024

View reviewed changes

rogpeppe force-pushed the 047-testscript-config-files branch 2 times, most recently from 9bd33c2 to 699fae6 Compare June 11, 2024 07:56

mvdan approved these changes Jun 11, 2024

View reviewed changes

rogpeppe force-pushed the 047-testscript-config-files branch from 699fae6 to f01ab65 Compare June 11, 2024 08:12

rogpeppe merged commit b143f3f into master Jun 11, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

testscript: add Config.Files #258

testscript: add Config.Files #258

rogpeppe commented Jun 10, 2024

abhinav left a comment

abhinav Jun 10, 2024

rogpeppe Jun 11, 2024

abhinav Jun 10, 2024

rogpeppe Jun 11, 2024

mvdan Jun 11, 2024

rogpeppe Jun 11, 2024

testscript: add Config.Files #258

testscript: add Config.Files #258

Conversation

rogpeppe commented Jun 10, 2024

abhinav left a comment

Choose a reason for hiding this comment

abhinav Jun 10, 2024

Choose a reason for hiding this comment

rogpeppe Jun 11, 2024

Choose a reason for hiding this comment

abhinav Jun 10, 2024

Choose a reason for hiding this comment

rogpeppe Jun 11, 2024

Choose a reason for hiding this comment

mvdan Jun 11, 2024

Choose a reason for hiding this comment

rogpeppe Jun 11, 2024

Choose a reason for hiding this comment