Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1774)

Issue 12796045: code review 12796045: go.tools/go/vcs: do not delete $TMPDIR during test runs (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by dfc
Modified:
10 years, 7 months ago
Reviewers:
r, adg, bradfitz
CC:
r, adg, golang-dev
Visibility:
Public.

Description

go.tools/go/vcs: do not delete $TMPDIR during test runs The test would nuke the entire contents of os.TempDir on completion. This change corrects the code to use ioutil.TempDir.

Patch Set 1 #

Patch Set 2 : diff -r a7b751825c27 https://code.google.com/p/go.tools #

Patch Set 3 : diff -r a7b751825c27 https://code.google.com/p/go.tools #

Patch Set 4 : diff -r a7b751825c27 https://code.google.com/p/go.tools #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -2 lines) Patch
M go/vcs/vcs_test.go View 1 3 chunks +6 lines, -2 lines 0 comments Download

Messages

Total messages: 7
dfc
Hello r@golang.org, adg@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.tools
10 years, 7 months ago (2013-08-19 06:18:50 UTC) #1
r
LGTM
10 years, 7 months ago (2013-08-19 06:20:17 UTC) #2
dfc
On 2013/08/19 06:18:50, dfc wrote: > Hello mailto:r@golang.org, mailto:adg@golang.org (cc: mailto:golang-dev@googlegroups.com), > > I'd like ...
10 years, 7 months ago (2013-08-19 06:21:44 UTC) #3
adg
LGTM wow
10 years, 7 months ago (2013-08-19 06:22:37 UTC) #4
dfc
*** Submitted as https://code.google.com/p/go/source/detail?r=3cd8b7bed301&repo=tools *** go.tools/go/vcs: do not delete $TMPDIR during test runs The test ...
10 years, 7 months ago (2013-08-19 06:22:58 UTC) #5
dfc
Depending on how you run your builder, you should logout/login or restart your machine to ...
10 years, 7 months ago (2013-08-19 06:24:00 UTC) #6
bradfitz
10 years, 7 months ago (2013-08-19 14:05:57 UTC) #7
Hah, nice.
 On Aug 18, 2013 11:18 PM, <dave@cheney.net> wrote:

> Reviewers: r, adg,
>
> Message:
> Hello r@golang.org, adg@golang.org (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go.**tools <https://code.google.com/p/go.tools>
>
>
> Description:
> go.tools/go/vcs: do not delete $TMPDIR during test runs
>
> The test would nuke the entire contents of os.TempDir on completion.
>
> This change corrects the code to use ioutil.TempDir.
>
> Please review this at
https://codereview.appspot.**com/12796045/<https://codereview.appspot.com/127...
>
> Affected files:
>   M go/vcs/vcs_test.go
>
>
> Index: go/vcs/vcs_test.go
> ==============================**==============================**=======
> --- a/go/vcs/vcs_test.go
> +++ b/go/vcs/vcs_test.go
> @@ -5,6 +5,7 @@
>  package vcs
>
>  import (
> +       "io/ioutil"
>         "os"
>         "path/filepath"
>         "testing"
> @@ -50,7 +51,11 @@
>         }
>
>         tests := make([]testStruct, len(vcsList))
> -       tempDir := os.TempDir()
> +       tempDir, err := ioutil.TempDir("", "vcstest")
> +       if err != nil {
> +               t.Fatal(err)
> +       }
> +       defer os.RemoveAll(tempDir)
>
>         for i, vcs := range vcsList {
>                 tests[i] = testStruct{
> @@ -67,5 +72,4 @@
>                 }
>                 os.RemoveAll(test.path)
>         }
> -       os.RemoveAll(tempDir)
>  }
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b