code review 7370049: net/http/cgi: move syscall usage in test to its own not...

net/http/cgi: make tests compile on plan9

Don't reference the non-portable syscall.Signal(0).

Maybe they'll pass too. Untested. plan9 bit from
Akshat Kumar.

[bradfitz, 2013-02-25 20:59:46]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[akumar_mail.nanosouffle.net, 2013-02-25 21:23:21]

LGTM.

It would be great if you could include a new file, plan9_test.go
with the following:

// Copyright 2013 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.

// +build plan9

package cgi

import (
	"os"
	"testing"
)

func isProcessRunning(t *testing.T, pid int) bool {
	_, err := os.Stat("/proc/" + itoa(pid))
	return err == nil
}

On 25 February 2013 20:59,  <bradfitz@golang.org> wrote:
> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> net/http/cgi: move syscall usage in test to its own not-plan9 file
>
> For the plan9 guys to finish fixing from here.
>
> Please review this at https://codereview.appspot.com/7370049/
>
> Affected files:
>   M src/pkg/net/http/cgi/host_test.go
>   A src/pkg/net/http/cgi/posix_test.go
>
>
> Index: src/pkg/net/http/cgi/host_test.go
> ===================================================================
> --- a/src/pkg/net/http/cgi/host_test.go
> +++ b/src/pkg/net/http/cgi/host_test.go
> @@ -19,7 +19,6 @@
>         "runtime"
>         "strconv"
>         "strings"
> -       "syscall"
>         "testing"
>         "time"
>  )
> @@ -340,11 +339,7 @@
>         }
>
>         childRunning := func() bool {
> -               p, err := os.FindProcess(pid)
> -               if err != nil {
> -                       return false
> -               }
> -               return p.Signal(syscall.Signal(0)) == nil
> +               return isProcessRunning(t, pid)
>         }
>
>         if !childRunning() {
> Index: src/pkg/net/http/cgi/posix_test.go
> ===================================================================
> new file mode 100644
> --- /dev/null
> +++ b/src/pkg/net/http/cgi/posix_test.go
> @@ -0,0 +1,21 @@
> +// Copyright 2013 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.
> +
> +// +build !plan9
> +
> +package cgi
> +
> +import (
> +       "os"
> +       "syscall"
> +       "testing"
> +)
> +
> +func isProcessRunning(t *testing.T, pid int) bool {
> +       p, err := os.FindProcess(pid)
> +       if err != nil {
> +               return false
> +       }
> +       return p.Signal(syscall.Signal(0)) == nil
> +}
>
>
> --
>
> ---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.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[bradfitz, 2013-02-25 21:27:18]

*** Submitted as https://code.google.com/p/go/source/detail?r=dabf3d5de8a7 ***

net/http/cgi: make tests compile on plan9

Don't reference the non-portable syscall.Signal(0).

Maybe they'll pass too. Untested. plan9 bit from
Akshat Kumar.

R=golang-dev, akumar
CC=golang-dev
https://codereview.appspot.com/7370049

[bradfitz, 2013-02-25 21:27:18]

done, thanks.

On Mon, Feb 25, 2013 at 1:22 PM, Akshat Kumar
<akumar@mail.nanosouffle.net>wrote:

> LGTM.
>
> It would be great if you could include a new file, plan9_test.go
> with the following:
>
> // Copyright 2013 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.
>
> // +build plan9
>
> package cgi
>
> import (
>         "os"
>         "testing"
> )
>
> func isProcessRunning(t *testing.T, pid int) bool {
>         _, err := os.Stat("/proc/" + itoa(pid))
>         return err == nil
> }
>
> On 25 February 2013 20:59,  <bradfitz@golang.org> wrote:
> > Reviewers: golang-dev_googlegroups.com,
> >
> > Message:
> > Hello golang-dev@googlegroups.com,
> >
> > I'd like you to review this change to
> > https://go.googlecode.com/hg/
> >
> >
> > Description:
> > net/http/cgi: move syscall usage in test to its own not-plan9 file
> >
> > For the plan9 guys to finish fixing from here.
> >
> > Please review this at https://codereview.appspot.com/7370049/
> >
> > Affected files:
> >   M src/pkg/net/http/cgi/host_test.go
> >   A src/pkg/net/http/cgi/posix_test.go
> >
> >
> > Index: src/pkg/net/http/cgi/host_test.go
> > ===================================================================
> > --- a/src/pkg/net/http/cgi/host_test.go
> > +++ b/src/pkg/net/http/cgi/host_test.go
> > @@ -19,7 +19,6 @@
> >         "runtime"
> >         "strconv"
> >         "strings"
> > -       "syscall"
> >         "testing"
> >         "time"
> >  )
> > @@ -340,11 +339,7 @@
> >         }
> >
> >         childRunning := func() bool {
> > -               p, err := os.FindProcess(pid)
> > -               if err != nil {
> > -                       return false
> > -               }
> > -               return p.Signal(syscall.Signal(0)) == nil
> > +               return isProcessRunning(t, pid)
> >         }
> >
> >         if !childRunning() {
> > Index: src/pkg/net/http/cgi/posix_test.go
> > ===================================================================
> > new file mode 100644
> > --- /dev/null
> > +++ b/src/pkg/net/http/cgi/posix_test.go
> > @@ -0,0 +1,21 @@
> > +// Copyright 2013 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.
> > +
> > +// +build !plan9
> > +
> > +package cgi
> > +
> > +import (
> > +       "os"
> > +       "syscall"
> > +       "testing"
> > +)
> > +
> > +func isProcessRunning(t *testing.T, pid int) bool {
> > +       p, err := os.FindProcess(pid)
> > +       if err != nil {
> > +               return false
> > +       }
> > +       return p.Signal(syscall.Signal(0)) == nil
> > +}
> >
> >
> > --
> >
> > ---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.
> > For more options, visit https://groups.google.com/groups/opt_out.
> >
> >
>

[akumar_mail.nanosouffle.net, 2013-02-25 21:38:32]

Thank you for fixing the itoa and for the speedy delivery.
I've tested and this passes on Linux and Plan 9.

On 25 February 2013 21:27,  <bradfitz@golang.org> wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=dabf3d5de8a7 ***
>
> net/http/cgi: make tests compile on plan9
>
> Don't reference the non-portable syscall.Signal(0).
>
> Maybe they'll pass too. Untested. plan9 bit from
> Akshat Kumar.
>
> R=golang-dev, akumar
> CC=golang-dev
> https://codereview.appspot.com/7370049
>
>
> https://codereview.appspot.com/7370049/