Skip to content

testing: surprising interaction of subtests with TempDir #46624

Description

@cespare

What version of Go are you using (go version)?

I tested with Go 1.16.4 and tip (e1fa260).

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

Here's a demo (playground):

package main

import (
	"fmt"
	"path/filepath"
	"testing"
)

func TestTempdir(t *testing.T) {
	testGlob(t)
	t.Run("x=[1,2,3]", func(t *testing.T) {
		testGlob(t)
	})
	t.Run("x=[]", func(t *testing.T) {
		testGlob(t)
	})
}

func testGlob(t *testing.T) {
	dir := t.TempDir()
	glob := filepath.Join(dir, "*.txt")
	fmt.Println("glob:", glob)
	if _, err := filepath.Glob(glob); err != nil {
		t.Fatal("glob error:", err)
	}
}

What did you expect to see?

It'd be nice if this just worked.

What did you see instead?

$ go test -v ./x
=== RUN   TestTempdir
glob: /tmp/TestTempdir1889981746/001/*.txt
=== RUN   TestTempdir/x=[1,2,3]
glob: /tmp/TestTempdir_x=[1,2,3]163097497/001/*.txt
=== RUN   TestTempdir/x=[]
glob: /tmp/TestTempdir_x=[]3868477592/001/*.txt
    x_test.go:24: glob error: syntax error in pattern
--- FAIL: TestTempdir (0.00s)
    --- PASS: TestTempdir/x=[1,2,3] (0.00s)
    --- FAIL: TestTempdir/x=[] (0.00s)
FAIL

When naming some subtest, you probably aren't considering that the name is going to become part of the path of any t.TempDir(). This can have some surprising interactions, such as this case where the name includes [], making any glob pattern inside the tempdir invalid.

I suspect there are other lurking surprises. For example, if you use a * inside the test name, that is passed on to os.MkdirTemp as-is where it has a special meaning (it indicates where the random number should be inserted). Not broken, but also not intended. Or your subtest name can include characters such as !"#$&'(),;<=>?[]^{|} and backtick which may require shell quoting.

Perhaps t.TempDir shouldn't name its directory after the name of the test, or else perhaps it should use a limited set of allowed symbols rather than trying to remove a few disallowed ones (like /).

(This is a continuation of the issues uncovered in #38465 -- cc @bradfitz @tklauser)

Activity

  1. tklauser commented on Jun 7, 2021

    @tklauser
    Member

    Perhaps t.TempDir shouldn't name its directory after the name of the test, or else perhaps it should use a limited set of allowed symbols rather than trying to remove a few disallowed ones (like /).

    I think it might still useful for the directory to somewhat resemble the name of the test. But I agree that trying to remove disallowed symbols is error-prone and might still break on certain platforms. Using a limited set of symbols (e.g. alphanumeric characters plus a few safe ones) sounds good to me.

  2. added
    NeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.
    on Jun 7, 2021
  3. added this to the Backlog milestone on Jun 7, 2021
  4. cherrymui commented on Jun 7, 2021

    @cherrymui
    Member
  5. ianlancetaylor commented on Jun 7, 2021

    @ianlancetaylor
    Contributor

    Yes, let's just drop unusual characters, or replace them with - or something.

  6. gopherbot commented on Jun 8, 2021

    @gopherbot
    Contributor

    Change https://golang.org/cl/326010 mentions this issue: testing: drop unusual characters from TempDir directory name

  7. added
    NeedsFixThe path to resolution is known, but the work has not been done.
    on Jun 16, 2021
  8. removed
    NeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.
    on Jun 16, 2021
  9. ianlancetaylor commented on Jan 16, 2022

    @ianlancetaylor
    Contributor

    @gopherbot Please open backport to 1.16.

  10. gopherbot commented on Jan 16, 2022

    @gopherbot
    Contributor

    Backport issue(s) opened: #50645 (for 1.16).

    Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

  11. ianlancetaylor commented on Jan 16, 2022

    @ianlancetaylor
    Contributor

    (The fix is already in 1.17.)

  12. gopherbot commented on Jan 16, 2022

    @gopherbot
    Contributor

    Change https://golang.org/cl/378914 mentions this issue: [release-branch.go1.16] testing: drop unusual characters from TempDir directory name

  13. locked and limited conversation to collaborators on Jan 16, 2023
  14. added a commit that references this issue on Apr 29, 2024
    97cee43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions