Skip to content

testing: add TB.Chdir #62516

Description

@kolyshkin

Sometimes a test need to call os.Chdir(). Here is a bare minimum implementation of what's needed from a test to do that:

        oldwd, err := Getwd()    
        if err != nil {
                t.Fatal(err)
        }
        if err := Chdir(dir); err != nil {
                t.Fatal(err)
        }
        t.Cleanup(func() {
                if err := Chdir(oldwd); err != nil {
                        // It's not safe to continue with tests if we can't get back to
                        // the original working directory.
                        panic(err)
                }
        })

The code above can be used as a test helper; in fact, this repository already contains at least 5 helpers similar to the one above:

  1. func chdir(t *testing.T, dir string) {
  2. func chtmpdir(t *testing.T) func() {
  3. func chdir(t *testing.T, dir string) {
  4. func chtmpdir(t *testing.T) (restore func()) {
  5. func chtmpdir(t *testing.T) func() {

In addition, there are a few in-line implementations of the same functionality, another implementation in golang.org/x/sys/unix and so on.

The problem with this (except for multiple implementations and re-implementations) is, tests that use it can not use t.Parallel. Currently, there is no way to ensure that.

The issue is very similar to one for os.Setenv (#41260, fixed by https://golang.org/cl/326790); thus the solution is also similar.

The proposal is to add a Chdir method to the testing package, which will take care about all of the above.

The implementation may look like this: https://go-review.googlesource.com/c/go/+/529895

Activity

  1. added this to the Proposal milestone on Sep 7, 2023
  2. moved this to Incoming in Proposalson Sep 7, 2023
  3. gopherbot commented on Sep 9, 2023

    @gopherbot
    Contributor

    Change https://go.dev/cl/526717 mentions this issue: testing: add Chdir

  4. gopherbot commented on Sep 20, 2023

    @gopherbot
    Contributor

    Change https://go.dev/cl/529895 mentions this issue: testing: add Chdir

  5. kolyshkin commented on May 15, 2024

    @kolyshkin
    ContributorAuthor

    @rsc anything I could do to move it forward?

  6. rsc commented on May 23, 2024

    @rsc
    Contributor

    If tests need to chdir often, then I do think it is worth doing well, in much the same way that t.Setenv did (takes care of undoing, also takes care of not being in parallel tests). That said, I am not sure whether tests need to chdir often enough.
    Skimming CL 529896, it seems telling that all the changes are in packages like os, path/filepath, syscall, which are specifically about directories. Tests for code that is not about directories would be more compelling. I will try to gather some data from the open source Go corpus.

  7. moved this from Incoming to Active in Proposalson May 23, 2024
  8. rsc commented on May 23, 2024

    @rsc
    Contributor

    This proposal has been added to the active column of the proposals project
    and will now be reviewed at the weekly proposal review meetings.
    — rsc for the proposal review group

  9. jimmyfrasche commented on May 23, 2024

    @jimmyfrasche
    Member

    I ran into this recently. If it is helpful: https://github.com/jimmyfrasche/autoreadme/blob/0fbe6087ce8309d209b6e9caf3621e781bc73fcb/autoreadme_test.go

    (The program works on go modules in git repos and it needs to run in a directory within such a module and update files in that tree. Normally I would try to mock such things out but that would have increased the complexity of both the code and the tests too much. It was simpler to just have the test cd into mock projects and run the program there and then check the results against golden output. Because the projects contain go.mod in testdata they're stored indirectly in .txtar archives that are expanded into a tmp dir which is not relevant to the issue at hand but explains why there's all those other things happening in the test code.)

  10. kolyshkin commented on May 26, 2024

    @kolyshkin
    ContributorAuthor

    This is needed to test any functionality/code that uses relative paths.

    You can find many examples of tests that use os.Chdir here: https://sourcegraph.com/search?q=context:global+file:_test.go%24+os.Chdir&patternType=keyword&sm=0

    Most of them do the same sequence as in this issue description (save cwd, chdir, defer chdir to saved). Most are not entirely correct as they should panic if Chdir to oldwd failed.

    None are preventing running tests using Chdir in parallel, although some warn about it, see eg https://github.com/moby/moby/blob/ceefb7d0b9c5b6fbd1ea7511592a4ddb28ec4821/pkg/idtools/idtools_unix_test.go#L210-L221). This is somewhat hard to implement as you'd need a wrapper around t.Parallel.

  11. rsc commented on May 31, 2024

    @rsc
    Contributor

    Took me longer than expected but I finished scanning the latest versions of each module in the proxy cache. Overall there are 8756664 _test.go files, and 23963 files mention os.Chdir, or about 0.27%. It's not high, but we didn't expect it to be that high. The next question is whether os.Chdir is really necessary in those tests. I sampled 20 at random (numbers 1..20 in the sampled results here) and found:

    • 1 unnecessary substitute for filepath.ResolveSymlinks (2)
    • 4 not strictly necessary but helpful (1, 7, 8, 15)
    • 15 necessary (3, 4, 5, 6, 9, 10, 11, 12, 13, 14, 16, 17, 18, 19, 20)

    And 5 (25%) forgot to chdir back, which could break other tests (5, 10, 12, 16, 18).

    It still doesn't come up often, but when it does it's difficult to get right. I think it is reasonable for the testing package to help, in the same way that it helps with os.Setenv.

  12. moved this from Active to Likely Accept in Proposalson Jun 12, 2024
  13. rsc commented on Jun 12, 2024

    @rsc
    Contributor

    Based on the discussion above, this proposal seems like a likely accept.
    — rsc for the proposal review group

    The proposal is to add a method Chdir(dir string) to to testing.T, testing.B, testing.TB.

    The method calls t/b.Fatal if the chdir fails.
    It also calls t/b.Fatal if called during a parallel test.
    It also makes sure to restore the previous current directory when the test finishes.
    Internally, it can use Open(".") and os.Fchdir.
    It should probably also call t.Setenv to set $PWD, like os/exec does when Dir is set.

  14. 6 remaining items

  15. added this to the Backlog milestone on Jun 24, 2024
  16. added a commit that references this issue on Aug 16, 2024
  17. gopherbot commented on Aug 20, 2024

    @gopherbot
    Contributor

    Change https://go.dev/cl/607035 mentions this issue: testing: use temp dir without symlinks in TestChdir/relative

  18. modified the milestones: Backlog, Go1.24 on Aug 20, 2024
  19. gopherbot commented on Aug 20, 2024

    @gopherbot
    Contributor

    Change https://go.dev/cl/607095 mentions this issue: doc: revert #62516 relnote accidentally modified in CL 603959

  20. mvdan commented on Nov 20, 2024

    @mvdan
    Member
  21. ianlancetaylor commented on Nov 20, 2024

    @ianlancetaylor
    Contributor

    I think it was left open to start using it in the standard library. But let's close it now. Thanks.

  22. locked and limited conversation to collaborators on Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions