Skip to content

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Sep 7, 2022

Summary

Add tests that check that devbox shell launches successfully on the platforms (Linux and macOS) and shells (bash, dash, and zsh) that we support. Specifically, each test runs the equivalent of typing the following into your terminal:

cd `mktemp -d`
devbox init
devbox add hello
devbox shell
hello
exit
exit

The bulk of this change is the addition of some test helpers that handle launching shells and interacting with them. For now they live in the same file as the boxcli shell tests, but we can move them into their own shelltest package later on if necessary. With these test helpers in place, it should be a lot easier to add tests for more complicated scenarios.

In order to run these tests in CI, the GitHub test jobs now install Nix along with some additional shells. They also run on both ubuntu-latest and macos-12 to make sure things work on both systems. The lint job also runs on Ubuntu and macOS to make sure that we don't inadvertently add platform-specific code that won't compile.

How was it tested?

go test

@gcurtis gcurtis force-pushed the gcurtis/tests branch 8 times, most recently from 679447c to 1f3f9aa Compare September 8, 2022 18:42
brew update
brew install dash zsh
- name: Install Nix
run: sh <(curl -L https://nixos.org/nix/install) --daemon
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does nix get installed every single time the test runs? Can we cache this somehow

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does install on every run. There might be a way to cache it, but I haven't looked into it yet.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ask because if we install nix every single time, devbox shell would be quite slow given we have to install the nix store?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of installing nix, could we use a base image that has nix pre-installed? (for example, the same one we use in the dockerfile)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nvm, I see you're purposefully trying to test macos.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installing Nix does slow things down a lot. With these tests the total CI time is about 5 minutes.

I want to look into caching the Nix and Homebrew packages with the cache action, but as a first step I wanted to get the tests working.

@gcurtis gcurtis force-pushed the gcurtis/tests branch 5 times, most recently from 1052675 to 187c509 Compare September 8, 2022 20:13
Add tests that check that `devbox shell` launches successfully on the
platforms (Linux and macOS) and shells (bash, dash, and zsh) that we
support. Specifically, each test runs the equivalent of typing the
following into your terminal:

	cd `mktemp -d`
	devbox init
	devbox add hello
	devbox shell
	hello
	exit
	exit

The bulk of this change is the addition of some test helpers that handle
launching shells and interacting with them. For now they live in the
same file as the `boxcli` shell tests, but we can move them into their
own `shelltest` package later on if necessary. With these test helpers
in place, it should be a lot easier to add tests for more complicated
scenarios.

In order to run these tests in CI, the GitHub test jobs now install Nix
along with some additional shells. They also run on both ubuntu-latest
and macos-12 to make sure things work on both systems. The lint job
also runs on Ubuntu and macOS to make sure that we don't inadvertently
add platform-specific code that won't compile.
@gcurtis gcurtis changed the title boxcli: add shell tests boxcli: add basic tests for devbox shell Sep 8, 2022
Copy link
Contributor

@loreto loreto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


func TestShellHelloBash(t *testing.T) { testShellHello(t, "bash") }
func TestShellHelloDash(t *testing.T) { testShellHello(t, "dash") }
func TestShellHelloZsh(t *testing.T) { testShellHello(t, "zsh") }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wanted to be really exhaustive, this could be the list:

  • Bash
  • Cmd
  • Dash
  • Elvish
  • Fish
  • Ion
  • Nushell
  • PowerShell
  • Tcsh
  • Xonsh
  • Zsh

This is based on the list that starship works with: https://starship.rs/guide/#%F0%9F%9A%80-installation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there are a bunch more we could support. It's a bit of work because we have to implement the detection and using a custom shellrc. I should probably add some documentation on which shells we officially test and support.

@gcurtis gcurtis merged commit d0e4c23 into main Sep 9, 2022
@gcurtis gcurtis deleted the gcurtis/tests branch September 9, 2022 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants