Fix integration tests #1853
Fix integration tests #1853
Conversation
Naatan
commented
May 6, 2022
•
Naatan
commented
 DX-925 Fix failing integration tests
|
This reverts commit d9f541b.
| @@ -180,6 +185,7 @@ func new(t *testing.T, retainDirs, updatePath bool, extraEnv ...string) *Session | |||
| constants.ProjectEnvVarName + "=", | |||
| constants.E2ETestEnvVarName + "=true", | |||
| constants.DisableUpdates + "=true", | |||
| constants.OptinUnstableEnvVarName + "=true", | |||
There was a problem hiding this comment.
Can we still overwrite this environment in a test? Just want to make sure we can test unstable commands not working if you haven't opted in
There was a problem hiding this comment.
Yep, to overwrite you just specify the env var with an empty value. Or really any value other than true.
| @@ -45,6 +45,7 @@ func init() { | |||
| if locale == "" { | |||
| cfg, err := config.New() | |||
| if err == nil { | |||
| defer cfg.Close() | |||
There was a problem hiding this comment.
Should we use the closer here so any close errors are logged?
There was a problem hiding this comment.
This is an init function, so logging may not be available. I'd rather not risk making this a bigger issue than it needs to be, especially considering long term we'd be getting rid of this init function anyway.
| deprecated, err := r.deprecation.Check() | ||
| if err != nil { | ||
| return nil, errs.Wrap(err, "Could not fetch deprecation information") | ||
| deprecated, ok := r.depPoller.ValueFromCache().(*graph.DeprecationInfo) |
There was a problem hiding this comment.
This won't panic if the value from ValueFromCache() is nil right? I'm pretty sure it won't. Might be something nice to confirm in the unit test for the poller
There was a problem hiding this comment.
I can't confirm it in the poller, because the value type is an interface and we can't reliably assert nil unless we know the type.
We're not doing anything with the returned value here, other than also return it. And as per the contract of the function it CAN return a nil, which isn't a new change.
| baseDir := filepath.Join(tempPath, constants.ToplevelInstallArchiveDir) | ||
| stateExec := filepath.Join(baseDir, installation.BinDirName, constants.StateCmd+osutils.ExeExt) |
There was a problem hiding this comment.
This should work with installation.StateExecFromDir and I'm pretty sure it was in the PR that moved appinfo. What changed?
There was a problem hiding this comment.
That function is for using information from the installed state tool. This test is using a generated payload. While those are similar things they are not the same.
I believe in this case it would fail because the payload has no install dir marker.
# Conflicts: # cmd/state-svc/internal/deprecation/deprecation.go # cmd/state-svc/internal/resolver/resolver.go # internal/constants/constants.go
Fix integration tests # Conflicts: # .github/workflows/build.yml # cmd/state-installer/installer.go # internal/locale/locales/en-us.yaml # internal/testhelpers/e2e/session.go # test/integration/upgen_int_test.go
