code review 125860043: go.crypto/ssh/test: skip tests during -short mode

go.crypto/ssh/test: skip tests during -short mode

This proposal effectively disables all the ssh/test tests when run with the -short flag supplied.

For developers and users of this package, there should be no change unless they are in the habbit of always supplying -short, which I belive is untrue.

For the CI dashboard the effect should be that these tests, which are really not portable enough to run reliably on all our various builders, are disabled.

[dave_cheney.net, 2014-08-08 00:49:13]

Hello adg@golang.org (cc: golang-codereviews@googlegroups.com),

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

[dave_cheney.net, 2014-08-08 00:56:33]

As a little background to this change. 

I added the ssh/test sub directory as an attempt at doing integration testing of
the ssh package against a real world ssh server. The goal was to isolate unit
test failures in the ssh package, from integration failures, and to some extent
this goal was achieved; you could run go test
code.google.com/p/go.crypto/ssh/... and see that the ssh package worked, but
there was a potential integration issue with your system sshd if ssh/test
failed.

However, in the wider context of this package's tests being run on all our CI
builders, where the entire subrepo is judged on a binary pass or fail basis,
I've come to believe that adding this package was a mistake. Because of that I
feel embarrassed that the go.crypto subrepo is always the most flaky and I have
not invested the time in tracking down and resolving each compatibility issue.

With that said, it is my feeling that CI builders should not be expected to
provide a particularly rich environment for builds to run in. I cite the recent
decision not to install git on the freebsd builders to fix an issue in the
go.blog repo.

So in that context the simplest solution would be to not run the ssh/test
integration tests during CI, and this proposal does just that.

[adg, 2014-08-08 01:04:51]

LGTM

[dave_cheney.net, 2014-08-08 01:42:54]

*** Submitted as
https://code.google.com/p/go/source/detail?r=41cd4647fccc&repo=crypto ***

go.crypto/ssh/test: skip tests during -short mode

This proposal effectively disables all the ssh/test tests when run with the
-short flag supplied.

For developers and users of this package, there should be no change unless they
are in the habbit of always supplying -short, which I belive is untrue.

For the CI dashboard the effect should be that these tests, which are really not
portable enough to run reliably on all our various builders, are disabled.

LGTM=adg
R=adg, agl, hanwen
CC=golang-codereviews
https://codereview.appspot.com/125860043

[minux, 2014-08-08 01:45:04]

Will this fix https://code.google.com/p/go/issues/detail?id=8489?


On Thu, Aug 7, 2014 at 8:56 PM, <dave@cheney.net> wrote:

> As a little background to this change.
>
> I added the ssh/test sub directory as an attempt at doing integration
> testing of the ssh package against a real world ssh server. The goal was
> to isolate unit test failures in the ssh package, from integration
> failures, and to some extent this goal was achieved; you could run go
> test code.google.com/p/go.crypto/ssh/... and see that the ssh package
> worked, but there was a potential integration issue with your system
> sshd if ssh/test failed.
>
> However, in the wider context of this package's tests being run on all
> our CI builders, where the entire subrepo is judged on a binary pass or
> fail basis, I've come to believe that adding this package was a mistake.
> Because of that I feel embarrassed that the go.crypto subrepo is always
> the most flaky and I have not invested the time in tracking down and
> resolving each compatibility issue.
>
> With that said, it is my feeling that CI builders should not be expected
> to provide a particularly rich environment for builds to run in. I cite
> the recent decision not to install git on the freebsd builders to fix an
> issue in the go.blog repo.
>
> So in that context the simplest solution would be to not run the
> ssh/test integration tests during CI, and this proposal does just that.
>
> https://codereview.appspot.com/125860043/
>
>
> --
> 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.
>

[dave_cheney.net, 2014-08-08 01:52:08]

That issue was one impetus for this change, but I'd like to do better for people
who run the tests manually. 



> On 8 Aug 2014, at 11:44, minux <minux@golang.org> wrote:
> 
> Will this fix https://code.google.com/p/go/issues/detail?id=8489?
> 
> 
>> On Thu, Aug 7, 2014 at 8:56 PM, <dave@cheney.net> wrote:
>> As a little background to this change.
>> 
>> I added the ssh/test sub directory as an attempt at doing integration
>> testing of the ssh package against a real world ssh server. The goal was
>> to isolate unit test failures in the ssh package, from integration
>> failures, and to some extent this goal was achieved; you could run go
>> test code.google.com/p/go.crypto/ssh/... and see that the ssh package
>> worked, but there was a potential integration issue with your system
>> sshd if ssh/test failed.
>> 
>> However, in the wider context of this package's tests being run on all
>> our CI builders, where the entire subrepo is judged on a binary pass or
>> fail basis, I've come to believe that adding this package was a mistake.
>> Because of that I feel embarrassed that the go.crypto subrepo is always
>> the most flaky and I have not invested the time in tracking down and
>> resolving each compatibility issue.
>> 
>> With that said, it is my feeling that CI builders should not be expected
>> to provide a particularly rich environment for builds to run in. I cite
>> the recent decision not to install git on the freebsd builders to fix an
>> issue in the go.blog repo.
>> 
>> So in that context the simplest solution would be to not run the
>> ssh/test integration tests during CI, and this proposal does just that.
>> 
>> https://codereview.appspot.com/125860043/
>> 
>> 
>> -- 
>> 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.
> 

[hanwen-google, 2014-08-08 08:07:47]

thanks for taking care of this. SSH has slipped off my radar of late.

On Fri, Aug 8, 2014 at 3:52 AM, Dave Cheney <dave@cheney.net> wrote:
> That issue was one impetus for this change, but I'd like to do better for
> people who run the tests manually.
>
>
>
> On 8 Aug 2014, at 11:44, minux <minux@golang.org> wrote:
>
> Will this fix https://code.google.com/p/go/issues/detail?id=8489?
>
>
> On Thu, Aug 7, 2014 at 8:56 PM, <dave@cheney.net> wrote:
>>
>> As a little background to this change.
>>
>> I added the ssh/test sub directory as an attempt at doing integration
>> testing of the ssh package against a real world ssh server. The goal was
>> to isolate unit test failures in the ssh package, from integration
>> failures, and to some extent this goal was achieved; you could run go
>> test code.google.com/p/go.crypto/ssh/... and see that the ssh package
>> worked, but there was a potential integration issue with your system
>> sshd if ssh/test failed.
>>
>> However, in the wider context of this package's tests being run on all
>> our CI builders, where the entire subrepo is judged on a binary pass or
>> fail basis, I've come to believe that adding this package was a mistake.
>> Because of that I feel embarrassed that the go.crypto subrepo is always
>> the most flaky and I have not invested the time in tracking down and
>> resolving each compatibility issue.
>>
>> With that said, it is my feeling that CI builders should not be expected
>> to provide a particularly rich environment for builds to run in. I cite
>> the recent decision not to install git on the freebsd builders to fix an
>> issue in the go.blog repo.
>>
>> So in that context the simplest solution would be to not run the
>> ssh/test integration tests during CI, and this proposal does just that.
>>
>> https://codereview.appspot.com/125860043/
>>
>>
>> --
>> 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.
>
>



-- 
Han-Wen Nienhuys
Google Munich
hanwen@google.com