FIx base config loading and add TLS integration test #141
Conversation
|
Note, this seems to add ~15s to Windows tests due to costs of (I am actively working on fixing the nix child process closing issue) EDIT: All statements above no longer apply |
cmd/temporalite/main_integ_test.go
Outdated
| return fmt.Errorf("wait failed: %w", err) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
I think it might be possible to simplify some of the test setup/teardown by using the CLI as a library. Right now this doesn't make me want to write more of these style tests 😓
Eg.
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
go func() {
if err := buildCLI().RunContext(ctx, []string{"start", "--ephemeral", "--config", "."}); err != nil {
t.Fatal(err)
}
}()
// Run some tests
// ...If we dropped that test into the tls example directory, I think it would work without any runtime shenanigans.
There was a problem hiding this comment.
👍 WIll try (of course shouldn't call Fatal from other goroutines)
There was a problem hiding this comment.
If we dropped that test into the tls example directory
Getting:
import "github.com/temporalio/temporalite/cmd/temporalite" is a program, not an importable package
Just gonna leave test in cmd dir for now. May not be worth abstracting buildCLI to another package yet
There was a problem hiding this comment.
Yeah I'd be up for moving the CLI to an internal package eventually. I guess we should be able to use a relative path to get to the tls example config in the mean time.
There was a problem hiding this comment.
Unfortunately I will be making a dynamic YAML because it is unreasonable for me to set the working directory of tests and use relative paths to the certs inside my config. Oh well.
There was a problem hiding this comment.
Done using temp dir and templated config. Probably fine for now, will want a more robust harness w/ helpers if we make more of these.
| for i := 0; i < 50; i++ { | ||
| if c, err = client.Dial(options); err == nil { | ||
| break | ||
| } | ||
| time.Sleep(100 * time.Millisecond) | ||
| } |
There was a problem hiding this comment.
Just double checking, is there an internal timeout/retry mechanism in the Go SDK we could rely on?
There was a problem hiding this comment.
Not for dialing any more than net.Dial. We fail eagerly. There is a lazy client, but just does the same thing before the first high-level call is made.
|
lgtm, please feel free to dismiss remaining questions if they're not actionable. |
What changed?
Why?