code review 168490043: test: write Go wrapper code so that bug302 can be run b...

test: write Go wrapper code so that bug302 can be run by run.go

Update issue 4139

[iant, 2014-11-11 02:08:17]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2014-11-11 13:44:02]

LGTM


On Mon, Nov 10, 2014 at 11:08 PM, <iant@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> test: write Go wrapper code so that bug302 can be run by run.go
>
> Please review this at https://codereview.appspot.com/168490043/
>
> Affected files (+42, -5 lines):
>   M test/fixedbugs/bug302.go
>   M test/run.go
>
>
> Index: test/fixedbugs/bug302.go
> ===================================================================
> --- a/test/fixedbugs/bug302.go
> +++ b/test/fixedbugs/bug302.go
> @@ -1,9 +1,47 @@
> -// $G $D/bug302.dir/p.go && pack grc pp.a p.$A && $G $D/bug302.dir/main.go
> -
> -// NOTE: This test is not run by 'run.go' and so not run by all.bash.
> -// To run this test you must use the ./run shell script.
> +// run
>
>  // Copyright 2010 The Go Authors.  All rights reserved.
>  // Use of this source code is governed by a BSD-style
>  // license that can be found in the LICENSE file.
>
> +package main
> +
> +import (
> +       "fmt"
> +       "os"
> +       "os/exec"
> +       "runtime"
> +)
> +
> +var thechar = map[string]string{
> +       "arm": "5",
> +       "amd64": "6",
> +       "amd64p32": "6",
> +       "386": "8",
> +       "power64": "9",
> +       "power64le": "9",
> +}
> +
> +func main() {
> +       a := thechar[runtime.GOARCH]
> +       if a == "" {
> +               fmt.Println("BUG: unknown GOARCH")
> +               os.Exit(1)
> +       }
> +
> +       run("go", "tool", a+"g", "fixedbugs/bug302.dir/p.go")
> +       run("go", "tool", "pack", "grc", "pp.a", "p."+a)
> +       run("go", "tool", a+"g", "fixedbugs/bug302.dir/main.go")
> +       os.Remove("p."+a)
> +       os.Remove("pp.a")
> +       os.Remove("main."+a)
> +}
> +
> +func run(cmd string, args ...string) {
> +       out, err := exec.Command(cmd, args...).CombinedOutput()
> +       if err != nil {
> +               fmt.Println(string(out))
> +               fmt.Println(err)
> +               os.Exit(1)
> +       }
> +}
> Index: test/run.go
> ===================================================================
> --- a/test/run.go
> +++ b/test/run.go
> @@ -908,7 +908,6 @@
>
>  var skipOkay = map[string]bool{
>         "fixedbugs/bug248.go": true, // combines errorcheckdir and rundir
> in the same dir.
> -       "fixedbugs/bug302.go": true, // tests both .$O and .a imports.
>         "fixedbugs/bug345.go": true, // needs the appropriate flags in gc
> invocation.
>         "fixedbugs/bug369.go": true, // needs compiler flags.
>         "fixedbugs/bug429.go": true, // like "run" but program should fail
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[iant, 2014-11-12 02:41:41]

I think I'll submit after the release, unless somebody wants it
earlier.  We do want this change, but the only way it could affect the
release would be negatively.

Ian

On Tue, Nov 11, 2014 at 5:44 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> LGTM
>
>
> On Mon, Nov 10, 2014 at 11:08 PM, <iant@golang.org> wrote:
>>
>> Reviewers: golang-codereviews,
>>
>> Message:
>> Hello golang-codereviews@googlegroups.com,
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go
>>
>>
>> Description:
>> test: write Go wrapper code so that bug302 can be run by run.go
>>
>> Please review this at https://codereview.appspot.com/168490043/
>>
>> Affected files (+42, -5 lines):
>>   M test/fixedbugs/bug302.go
>>   M test/run.go
>>
>>
>> Index: test/fixedbugs/bug302.go
>> ===================================================================
>> --- a/test/fixedbugs/bug302.go
>> +++ b/test/fixedbugs/bug302.go
>> @@ -1,9 +1,47 @@
>> -// $G $D/bug302.dir/p.go && pack grc pp.a p.$A && $G
>> $D/bug302.dir/main.go
>> -
>> -// NOTE: This test is not run by 'run.go' and so not run by all.bash.
>> -// To run this test you must use the ./run shell script.
>> +// run
>>
>>  // Copyright 2010 The Go Authors.  All rights reserved.
>>  // Use of this source code is governed by a BSD-style
>>  // license that can be found in the LICENSE file.
>>
>> +package main
>> +
>> +import (
>> +       "fmt"
>> +       "os"
>> +       "os/exec"
>> +       "runtime"
>> +)
>> +
>> +var thechar = map[string]string{
>> +       "arm": "5",
>> +       "amd64": "6",
>> +       "amd64p32": "6",
>> +       "386": "8",
>> +       "power64": "9",
>> +       "power64le": "9",
>> +}
>> +
>> +func main() {
>> +       a := thechar[runtime.GOARCH]
>> +       if a == "" {
>> +               fmt.Println("BUG: unknown GOARCH")
>> +               os.Exit(1)
>> +       }
>> +
>> +       run("go", "tool", a+"g", "fixedbugs/bug302.dir/p.go")
>> +       run("go", "tool", "pack", "grc", "pp.a", "p."+a)
>> +       run("go", "tool", a+"g", "fixedbugs/bug302.dir/main.go")
>> +       os.Remove("p."+a)
>> +       os.Remove("pp.a")
>> +       os.Remove("main."+a)
>> +}
>> +
>> +func run(cmd string, args ...string) {
>> +       out, err := exec.Command(cmd, args...).CombinedOutput()
>> +       if err != nil {
>> +               fmt.Println(string(out))
>> +               fmt.Println(err)
>> +               os.Exit(1)
>> +       }
>> +}
>> Index: test/run.go
>> ===================================================================
>> --- a/test/run.go
>> +++ b/test/run.go
>> @@ -908,7 +908,6 @@
>>
>>  var skipOkay = map[string]bool{
>>         "fixedbugs/bug248.go": true, // combines errorcheckdir and rundir
>> in the same dir.
>> -       "fixedbugs/bug302.go": true, // tests both .$O and .a imports.
>>         "fixedbugs/bug345.go": true, // needs the appropriate flags in gc
>> invocation.
>>         "fixedbugs/bug369.go": true, // needs compiler flags.
>>         "fixedbugs/bug429.go": true, // like "run" but program should fail
>>
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "golang-codereviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-codereviews+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>

[bradfitz, 2014-11-12 17:15:07]

It's just a test. If it breaks a builder we can fix or roll back. I'd like
to see the test/run shell script deleted.

But later is fine too.



On Tue, Nov 11, 2014 at 11:41 PM, Ian Taylor <iant@golang.org> wrote:

> I think I'll submit after the release, unless somebody wants it
> earlier.  We do want this change, but the only way it could affect the
> release would be negatively.
>
> Ian
>
> On Tue, Nov 11, 2014 at 5:44 AM, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> > LGTM
> >
> >
> > On Mon, Nov 10, 2014 at 11:08 PM, <iant@golang.org> wrote:
> >>
> >> Reviewers: golang-codereviews,
> >>
> >> Message:
> >> Hello golang-codereviews@googlegroups.com,
> >>
> >> I'd like you to review this change to
> >> https://code.google.com/p/go
> >>
> >>
> >> Description:
> >> test: write Go wrapper code so that bug302 can be run by run.go
> >>
> >> Please review this at https://codereview.appspot.com/168490043/
> >>
> >> Affected files (+42, -5 lines):
> >>   M test/fixedbugs/bug302.go
> >>   M test/run.go
> >>
> >>
> >> Index: test/fixedbugs/bug302.go
> >> ===================================================================
> >> --- a/test/fixedbugs/bug302.go
> >> +++ b/test/fixedbugs/bug302.go
> >> @@ -1,9 +1,47 @@
> >> -// $G $D/bug302.dir/p.go && pack grc pp.a p.$A && $G
> >> $D/bug302.dir/main.go
> >> -
> >> -// NOTE: This test is not run by 'run.go' and so not run by all.bash.
> >> -// To run this test you must use the ./run shell script.
> >> +// run
> >>
> >>  // Copyright 2010 The Go Authors.  All rights reserved.
> >>  // Use of this source code is governed by a BSD-style
> >>  // license that can be found in the LICENSE file.
> >>
> >> +package main
> >> +
> >> +import (
> >> +       "fmt"
> >> +       "os"
> >> +       "os/exec"
> >> +       "runtime"
> >> +)
> >> +
> >> +var thechar = map[string]string{
> >> +       "arm": "5",
> >> +       "amd64": "6",
> >> +       "amd64p32": "6",
> >> +       "386": "8",
> >> +       "power64": "9",
> >> +       "power64le": "9",
> >> +}
> >> +
> >> +func main() {
> >> +       a := thechar[runtime.GOARCH]
> >> +       if a == "" {
> >> +               fmt.Println("BUG: unknown GOARCH")
> >> +               os.Exit(1)
> >> +       }
> >> +
> >> +       run("go", "tool", a+"g", "fixedbugs/bug302.dir/p.go")
> >> +       run("go", "tool", "pack", "grc", "pp.a", "p."+a)
> >> +       run("go", "tool", a+"g", "fixedbugs/bug302.dir/main.go")
> >> +       os.Remove("p."+a)
> >> +       os.Remove("pp.a")
> >> +       os.Remove("main."+a)
> >> +}
> >> +
> >> +func run(cmd string, args ...string) {
> >> +       out, err := exec.Command(cmd, args...).CombinedOutput()
> >> +       if err != nil {
> >> +               fmt.Println(string(out))
> >> +               fmt.Println(err)
> >> +               os.Exit(1)
> >> +       }
> >> +}
> >> Index: test/run.go
> >> ===================================================================
> >> --- a/test/run.go
> >> +++ b/test/run.go
> >> @@ -908,7 +908,6 @@
> >>
> >>  var skipOkay = map[string]bool{
> >>         "fixedbugs/bug248.go": true, // combines errorcheckdir and
> rundir
> >> in the same dir.
> >> -       "fixedbugs/bug302.go": true, // tests both .$O and .a imports.
> >>         "fixedbugs/bug345.go": true, // needs the appropriate flags in
> gc
> >> invocation.
> >>         "fixedbugs/bug369.go": true, // needs compiler flags.
> >>         "fixedbugs/bug429.go": true, // like "run" but program should
> fail
> >>
> >>
> >>
> >> --
> >> You received this message because you are subscribed to the Google
> Groups
> >> "golang-codereviews" group.
> >> To unsubscribe from this group and stop receiving emails from it, send
> an
> >> email to golang-codereviews+unsubscribe@googlegroups.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >
> >
>

[josharian, 2014-12-05 01:19:29]

FWIW, CL 147220043 covers bug 302 and the rest of the old-style tests. I'll
update it and re-mail when the tree opens for 1.5.

[gobot, 2014-12-19 05:15:23]

R=close

To the author of this CL:

The Go project has moved to Gerrit Code Review.

If this CL should be continued, please see the latest version of
https://golang.org/doc/contribute.html for instructions on
how to set up Git and the Go project's Gerrit codereview plugin,
and then create a new change with your current code.

If there has been discussion on this CL, please give a link to it
(golang.org/cl/168490043 is best) in the description in your
new CL.

Thanks very much.