Skip to content

Commit

Permalink
[flakes] ensure all golang tests pass when flakes flag is enabled (#697)
Browse files Browse the repository at this point in the history
## Summary

Needed to make two fixes:

1. `RmTest`:
here, the test was failing because with flakes flag we now check that
the user
has the package installed previously and so we do `devbox run <command>`
to trigger
package installation as a side-effect.

2. `ShellTest`:
Flakes no longer works with there being no default shell, so we call
that out. But
in a subsequent PR, I'll revisit this since I think we can get rid of
this block.
Wanted to keep the scope of this PR to just enabling these tests to
pass.

## How was it tested?

enabled the Flakes feature flag in the code and ran `go test ./...`
  • Loading branch information
savil committed Feb 27, 2023
1 parent 999dcf6 commit e5aaf97
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 9 deletions.
2 changes: 1 addition & 1 deletion examples/testdata/go/go-1.19/devbox.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
"nixpkgs": {
"commit": "52e3e80afff4b16ccb7c52e9f0f5220552f03d04"
}
}
}
19 changes: 14 additions & 5 deletions internal/boxcli/rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,29 @@ func TestRm(t *testing.T) {
"hello"
],
"shell": {
"scripts": {
"test1": "echo test1"
},
"init_hook": null
},
"nixpkgs": {
"commit": "af9e00071d0971eb292fd5abef334e66eda3cb69"
}
}`
td := testframework.Open()
defer td.Close()
err := td.SetDevboxJSON(devboxJSON)
testbox := testframework.Open()
defer testbox.Close()
err := testbox.SetDevboxJSON(devboxJSON)
assert.NoError(t, err)
output, err := td.RunCommand(RemoveCmd(), "hello")

// First, run a devbox script to install the packages as a side-effect
_, err = testbox.RunCommand(RunCmd(), "test1")
assert.NoError(t, err)

// Now, run the Remove command
output, err := testbox.RunCommand(RemoveCmd(), "hello")
assert.NoError(t, err)
assert.Contains(t, output, "hello (hello-2.12.1) is now removed.")
devboxjson, err := td.GetDevboxJSON()
devboxjson, err := testbox.GetDevboxJSON()
assert.NoError(t, err)
assert.NotContains(t, devboxjson.RawPackages, "hello")
}
13 changes: 11 additions & 2 deletions internal/boxcli/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"time"

"github.com/stretchr/testify/assert"
"go.jetpack.io/devbox/internal/boxcli/featureflag"
"go.jetpack.io/devbox/internal/nix"
"go.jetpack.io/devbox/internal/testframework"
)

Expand Down Expand Up @@ -311,6 +313,13 @@ func TestShell(t *testing.T) {
err := td.SetDevboxJSON(devboxJSON)
assert.NoError(t, err)
output, err := td.RunCommand(ShellCmd())
assert.NoError(t, err)
assert.Contains(t, output, "Starting a devbox shell...")
if featureflag.Flakes.Enabled() {
assert.Error(t, err)
if !errors.Is(err, nix.ErrNoDefaultShellUnsupportedInFlakesMode) {
assert.Fail(t, "Expected error %s but received %s", nix.ErrNoDefaultShellUnsupportedInFlakesMode, err)
}
} else {
assert.NoError(t, err)
assert.Contains(t, output, "Starting a devbox shell...")
}
}
6 changes: 5 additions & 1 deletion internal/nix/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const (
shPosix name = "posix"
)

var ErrNoDefaultShellUnsupportedInFlakesMode = errors.New("No default shell not supported in Flakes mode")

// Shell configures a user's shell to run in Devbox. Its zero value is a
// fallback shell that launches a regular Nix shell.
type Shell struct {
Expand Down Expand Up @@ -205,8 +207,10 @@ func (s *Shell) Run(nixShellFilePath, nixFlakesFilePath string) error {
// Launch a fallback shell if we couldn't find the path to the user's
// default shell.
if s.binPath == "" {

// TODO savil: fix this.
if featureflag.Flakes.Enabled() {
return errors.New("No default shell not supported in Flakes mode")
return ErrNoDefaultShellUnsupportedInFlakesMode
}
cmd := exec.Command("nix-shell", "--pure")
cmd.Args = append(cmd.Args, toKeepArgs(env, buildAllowList(s.env))...)
Expand Down

0 comments on commit e5aaf97

Please sign in to comment.