code review 69870045: sync/atomic: skip broken tests on freebsd/arm and netbsd/arm

sync/atomic: skip broken tests on freebsd/arm and netbsd/arm

Update issue 7338

The nil deref tests are currently failing on the *bsd/arm platforms. In an effort to avoid the build deteriorating further I would like to skip these tests on freebsd/arm and netbsd/arm.

[dave_cheney.net, 2014-03-01 08:25:29]

Hello golang-codereviews@googlegroups.com,

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

[bradfitz, 2014-03-01 17:37:19]

I'd rather see it fixed.

Papering over something as low-level as sync/atomic will just cause
mysterious problems elsewhere and wasted debugging time.

At least now we know where the problem is.



On Sat, Mar 1, 2014 at 12:25 AM, <dave@cheney.net> 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:
> sync/atomic: skip broken tests on freebsd/arm and netbsd/arm
>
> Update issue 7338
>
> The nil deref tests are currently failing on the *bsd/arm platforms. In
> an effort to avoid the build deteriorating further I would like to skip
> these tests on freebsd/arm and netbsd/arm.
>
> Please review this at https://codereview.appspot.com/69870045/
>
> Affected files (+3, -0 lines):
>   M src/pkg/sync/atomic/atomic_test.go
>
>
> Index: src/pkg/sync/atomic/atomic_test.go
> ===================================================================
> --- a/src/pkg/sync/atomic/atomic_test.go
> +++ b/src/pkg/sync/atomic/atomic_test.go
> @@ -1463,6 +1463,9 @@
>  }
>
>  func TestNilDeref(t *testing.T) {
> +       if p := runtime.GOOS + "/" + runtime.GOARCH; p == "freebsd/arm" ||
> p == "netbsd/arm" {
> +               t.Skipf("issue 7338: skipping test on %q", p)
> +       }
>         funcs := [...]func(){
>                 func() { CompareAndSwapInt32(nil, 0, 0) },
>                 func() { CompareAndSwapInt64(nil, 0, 0) },
>
>
> --
> 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/groups/opt_out.
>

[bradfitz, 2014-03-01 17:40:16]

LGTM

Oh, sorry, I hadn't read the bug or TestNilDeref yet.  I thought this was a
larger problem.  LGTM considering there's a note to the bug and it's
flagged Go1.3 already and Minux says he's on it.



On Sat, Mar 1, 2014 at 9:37 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> I'd rather see it fixed.
>
> Papering over something as low-level as sync/atomic will just cause
> mysterious problems elsewhere and wasted debugging time.
>
> At least now we know where the problem is.
>
>
>
> On Sat, Mar 1, 2014 at 12:25 AM, <dave@cheney.net> 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:
>> sync/atomic: skip broken tests on freebsd/arm and netbsd/arm
>>
>> Update issue 7338
>>
>> The nil deref tests are currently failing on the *bsd/arm platforms. In
>> an effort to avoid the build deteriorating further I would like to skip
>> these tests on freebsd/arm and netbsd/arm.
>>
>> Please review this at https://codereview.appspot.com/69870045/
>>
>> Affected files (+3, -0 lines):
>>   M src/pkg/sync/atomic/atomic_test.go
>>
>>
>> Index: src/pkg/sync/atomic/atomic_test.go
>> ===================================================================
>> --- a/src/pkg/sync/atomic/atomic_test.go
>> +++ b/src/pkg/sync/atomic/atomic_test.go
>> @@ -1463,6 +1463,9 @@
>>  }
>>
>>  func TestNilDeref(t *testing.T) {
>> +       if p := runtime.GOOS + "/" + runtime.GOARCH; p == "freebsd/arm"
>> || p == "netbsd/arm" {
>> +               t.Skipf("issue 7338: skipping test on %q", p)
>> +       }
>>         funcs := [...]func(){
>>                 func() { CompareAndSwapInt32(nil, 0, 0) },
>>                 func() { CompareAndSwapInt64(nil, 0, 0) },
>>
>>
>> --
>> 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/groups/opt_out.
>>
>
>

[dave_cheney.net, 2014-03-01 21:14:51]

Thanks Brad. Comments from anyone else?


On Sun, Mar 2, 2014 at 4:40 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> LGTM
>
> Oh, sorry, I hadn't read the bug or TestNilDeref yet.  I thought this was
> a larger problem.  LGTM considering there's a note to the bug and it's
> flagged Go1.3 already and Minux says he's on it.
>
>
>
> On Sat, Mar 1, 2014 at 9:37 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote:
>
>> I'd rather see it fixed.
>>
>> Papering over something as low-level as sync/atomic will just cause
>> mysterious problems elsewhere and wasted debugging time.
>>
>> At least now we know where the problem is.
>>
>>
>>
>> On Sat, Mar 1, 2014 at 12:25 AM, <dave@cheney.net> 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:
>>> sync/atomic: skip broken tests on freebsd/arm and netbsd/arm
>>>
>>> Update issue 7338
>>>
>>> The nil deref tests are currently failing on the *bsd/arm platforms. In
>>> an effort to avoid the build deteriorating further I would like to skip
>>> these tests on freebsd/arm and netbsd/arm.
>>>
>>> Please review this at https://codereview.appspot.com/69870045/
>>>
>>> Affected files (+3, -0 lines):
>>>   M src/pkg/sync/atomic/atomic_test.go
>>>
>>>
>>> Index: src/pkg/sync/atomic/atomic_test.go
>>> ===================================================================
>>> --- a/src/pkg/sync/atomic/atomic_test.go
>>> +++ b/src/pkg/sync/atomic/atomic_test.go
>>> @@ -1463,6 +1463,9 @@
>>>  }
>>>
>>>  func TestNilDeref(t *testing.T) {
>>> +       if p := runtime.GOOS + "/" + runtime.GOARCH; p == "freebsd/arm"
>>> || p == "netbsd/arm" {
>>> +               t.Skipf("issue 7338: skipping test on %q", p)
>>> +       }
>>>         funcs := [...]func(){
>>>                 func() { CompareAndSwapInt32(nil, 0, 0) },
>>>                 func() { CompareAndSwapInt64(nil, 0, 0) },
>>>
>>>
>>> --
>>> 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/groups/opt_out.
>>>
>>
>>
>

[minux1, 2014-03-01 21:21:58]

LGTM.

[dave_cheney.net, 2014-03-01 21:23:19]

Thanks, submitting, hopefully this gets us closer to a passing builder.


On Sun, Mar 2, 2014 at 8:21 AM, minux <minux.ma@gmail.com> wrote:

> LGTM.
>

[dave_cheney.net, 2014-03-01 21:32:26]

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

sync/atomic: skip broken tests on freebsd/arm and netbsd/arm

Update issue 7338

The nil deref tests are currently failing on the *bsd/arm platforms. In an
effort to avoid the build deteriorating further I would like to skip these tests
on freebsd/arm and netbsd/arm.

LGTM=bradfitz, minux.ma
R=golang-codereviews, bradfitz, minux.ma
CC=golang-codereviews
https://codereview.appspot.com/69870045

[gobot, 2014-03-01 21:35:34]

This CL appears to have broken the solaris-amd64-solaris11 builder.