provide and use compiled in test certs (only) in tests #757
Conversation
| @@ -28,7 +28,8 @@ import ( | |||
| ) | |||
|
|
|||
| // This is just the mechanics of certs generation. | |||
| func TestGenerateCerts(t *testing.T) { | |||
| // TODO(tschottdorf) make this test use the actual file system. | |||
| func disabledTestGenerateCerts(t *testing.T) { | |||
There was a problem hiding this comment.
shouldn't this work is you call ResetReadFileFn?
There was a problem hiding this comment.
yes, hence the todo. It's done now.
tbg
force-pushed
the
tschottdorf/mock_certs
branch
2 times, most recently
from
757d725 to
8c927c7
Compare
| @@ -49,8 +48,8 @@ func (c cliTest) Run(line string) { | |||
| var args []string | |||
| args = append(args, a[0]) | |||
| args = append(args, fmt.Sprintf("-addr=%s", c.ServingAddr())) | |||
| // Always load server certs. Not sure if this path is a good assumption though. | |||
| args = append(args, fmt.Sprintf("-certs=%s", security.EmbeddedPrefix+"test_certs")) | |||
| // Always load test certs. Not sure if this path is a good assumption though. | |||
There was a problem hiding this comment.
you can remove the part about the path, that was when I used ${GOPATH}/src/...
tbg
force-pushed
the
tschottdorf/mock_certs
branch
from
8c927c7 to
7c928b2
Compare
|
PTAL @mberhault |
tbg
changed the title
|
LGTM. assuming no test runners were missed. |
this allows tests to run without external dependencies, while avoiding compiled-in testdata in the main build.
tbg
force-pushed
the
tschottdorf/mock_certs
branch
from
7c928b2 to
a022f35
Compare
provide and use compiled in test certs (only) in tests
| @@ -26,7 +26,7 @@ import ( | |||
|
|
|||
| // NewTestContext returns a rpc.Context for testing. | |||
| func NewTestContext(t *testing.T) *Context { | |||
| tlsConfig, err := security.LoadTLSConfigFromDir(security.EmbeddedPrefix + "test_certs") | |||
| tlsConfig, err := security.LoadTLSConfigFromDir("test_certs") | |||
There was a problem hiding this comment.
Instead of leaking this constant "test_certs" directory, why not just have another security method called: security.LoadTLSConfigForTests? Then in security:
func LoadTLSConfigForTests() (*TLSConfig, error) {
return LoadTLSConfigFromDir("test_certs")
}
There was a problem hiding this comment.
Or make it an exported constant in the security package.
There was a problem hiding this comment.
I'm currently refactoring some of this with much fewer TLS setups. I'll move the test path to an exported const.
There was a problem hiding this comment.
@mberhault is refactoring the stuff anyways, I'll wait for the dust to settle. What's more worrying is that we currently do compile in the test certs everywhere, because it's used in the rpc/testing package. Will change that though.
this allows tests to run without external dependencies while avoiding compiled-in testdata in the main build.