code review 63530043: gosshnew/ssh: Move test data to testdata directory. (issue 63530043)

[jps...@google.com]

Reviewers: hanwen-google, agl1,

Message:
Hello han...@google.com, a...@golang.org (cc: a...@golang.org,
da...@cheney.net, golang-co...@googlegroups.com, han...@google.com,
jps...@google.com),

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


Description:
gosshnew/ssh: Move test data to testdata directory.

Please review this at https://codereview.appspot.com/63530043/

Affected files (+226, -362 lines):
M ssh/agent/client_test.go
R ssh/agent/testdata/id_dsa
R ssh/agent/testdata/id_ecdsa
R ssh/agent/testdata/id_rsa
A ssh/agent/testkeys_test.go
M ssh/client_auth_test.go
M ssh/keys_test.go
M ssh/test/agent_unix_test.go
M ssh/test/cert_test.go
R ssh/test/keys_test.go
M ssh/test/test_unix_test.go
A ssh/test/testkeys_test.go
A ssh/testdata/id_dsa
A ssh/testdata/id_dsa.pub
A ssh/testdata/id_ecdsa
A ssh/testdata/id_ecdsa.pub
A ssh/testdata/id_rsa
A ssh/testdata/id_rsa.pub
A ssh/testdata/id_user
A ssh/testdata/id_user.pub
A ssh/testdata/sshd_config
A ssh/testkeys_test.go

[jps...@google.com]

I'm not a fan of the duplication of testdata_test.go. I'm considering
making testdata a package, and having it export the objects directly.
Thoughts?

https://codereview.appspot.com/63530043/

[han...@google.com]

https://codereview.appspot.com/63530043/diff/20001/ssh/test/test_unix_test.go
File ssh/test/test_unix_test.go (right):

https://codereview.appspot.com/63530043/diff/20001/ssh/test/test_unix_test.go#newcode27
ssh/test/test_unix_test.go:27: var configTmpl =
template.Must(template.ParseFiles("../testdata/sshd_config"))
I think this is over the top - the sshd config is only useful for this
directory.

https://codereview.appspot.com/63530043/diff/20001/ssh/testkeys_test.go
File ssh/testkeys_test.go (right):

https://codereview.appspot.com/63530043/diff/20001/ssh/testkeys_test.go#newcode32
ssh/testkeys_test.go:32: rawRSAKey, rsaKey =
loadPrivateKey("id_rsa")
Let's just export the symbol? Since it's inside the testdata package
there is no downside.

https://codereview.appspot.com/63530043/

[jps...@google.com]

https://codereview.appspot.com/63530043/diff/20001/ssh/testkeys_test.go
File ssh/testkeys_test.go (right):

https://codereview.appspot.com/63530043/diff/20001/ssh/testkeys_test.go#newcode32
ssh/testkeys_test.go:32: rawRSAKey, rsaKey =
loadPrivateKey("id_rsa")

On 2014/02/14 09:02:44, hanwen-google wrote:
> Let's just export the symbol? Since it's inside the testdata package
there is no
> downside.

This is in the ssh package.

https://codereview.appspot.com/63530043/

[han...@google.com]

I'm not a fan of using files: they introduce extra boilerplate, because
reading them back from disk can fail, and need extra work to make work
if the test is not run from the source dir. Also, sshd may enforce
permissions for private keys so you have to read and write them to a
different file anyway.

Why not declare a bunch of exported constants in a testdata package,

var RSAKey = []byte("--RSA PRIVATE KEY--\n....")

then you can access them without having to go through the operating
system, and we can still share all the binary data between different
directories.



https://codereview.appspot.com/63530043/diff/20001/ssh/testdata/id_rsa
File ssh/testdata/id_rsa (right):

https://codereview.appspot.com/63530043/diff/20001/ssh/testdata/id_rsa#newcode1
ssh/testdata/id_rsa:1: -----BEGIN RSA PRIVATE KEY-----
if you use hg mv, the diffs will be shown correctly.

https://codereview.appspot.com/63530043/

[jps...@google.com]

*** Abandoned ***

https://codereview.appspot.com/63530043/