Skip to content

Conversation

savil
Copy link
Collaborator

@savil savil commented Mar 22, 2023

Summary

Why
Our goal is to make each Devbox release higher-quality by ensuring that all the devbox-projects in examples/ are able to run.

What
We aim to do so by adding a run_test devbox script to each project in examples/, and ensuring that it can run. This has the additional benefit of making the devbox.json self-documenting for any user who is reading the code in the examples/ directory: they can refer to the run_test to understand how to execute that example project.

How
To do so in our automated tests:

  1. We will generate a simple testscript for each of these devbox-projects that calls
    devbox run run_test.
  2. We copy the devbox project's files and directories into the testscript's workdir.

As a proof of concept, this PR runs a testscript for the examples/development/go/hello-world project. A future PR will generalize this to all of the devbox projects in examples/

How was it tested?

Ran with this command:

DEVBOX_DEBUG=0 go test -run TestExamples/development/go/hello-world -count 1 ./testscripts

Copy link
Collaborator Author

savil commented Mar 22, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@savil savil changed the title [testscripts] generate testscripts for a single devbox example project [testscripts] generate testscript for a single devbox example project Mar 22, 2023
@savil savil changed the title [testscripts] generate testscript for a single devbox example project [Lagobot 2.0] generate testscript for a single devbox example project Mar 22, 2023
@savil savil force-pushed the savil/examples-testscript-prototype branch 6 times, most recently from 4e0d10a to 104c240 Compare March 22, 2023 17:49
@savil savil requested a review from loreto March 22, 2023 17:50
@savil savil marked this pull request as ready for review March 22, 2023 17:52
@savil savil requested review from gcurtis and mikeland73 March 22, 2023 23:59
@savil savil changed the title [Lagobot 2.0] generate testscript for a single devbox example project [Lagobot 2.0] part 1: generate testscript for a single devbox example project Mar 23, 2023
Copy link
Collaborator

@gcurtis gcurtis left a comment

Choose a reason for hiding this comment

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

I really like the idea of having a run-test script in each example's devbox.json, but I'm not 100% clear on what testscripts is being used for. Couldn't we just invoke devbox run run_test with an empty-ish environment directly?

Comment on lines +63 to +68
// Golang caches build outputs for reuse in future builds in GOCACHE
goBuildCache := filepath.Join(cacheHome, "go-build")
env.Setenv("GOCACHE", goBuildCache)
if err = os.MkdirAll(goBuildCache, 0755); err != nil {
return err
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need to be set instead of using the default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

without this I get the following error:

failed to initialize build cache at /no-home/Library/Caches/go-build: mkdir /no-home: read-only file system

its because GOCACHE is likely set to $HOME/Library/Caches/go-build and testscripts sets $HOME to /no-home.

// RunExamplesTestscripts generates testscripts for each example devbox-project.
// For now, we prototype with the "go" example project. TODO savil: generalize.
func RunExamplesTestscripts(t *testing.T, examplesDir string) {
wd, err := os.Getwd()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Go always runs tests from the package's directory, so you should be able to just refer to ../examples directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, I see. I drop this in the part 2 PR then (to simplify merging).

Comment on lines +53 to +56
// implementation detail: the period at the end of the projectDir/.
// is important to ensure this works for both mac and linux.
// Ref.https://dev.to/ackshaey/macos-vs-linux-the-cp-command-will-trip-you-up-2p00
err = exec.Command("cp", "-r", projectDir+"/.", env.WorkDir).Run()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you run cp in run_test.test.txt and then not have to worry about the platform differences?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm wouldn't the cp command still come from the underlying platform?

}
}()

// create a temp-dir to place the testscript file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't testscript run the test in a temp dir?

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, which is the workDir we refer to elsewhere in this PR.

What this is doing is generating a directory that holds the "project that has to be tested", which would normally have the .test.txt file. I could simplify this by generating a single temp-dir, instead of re-generating this temp-dir for each example (as I do in part 2 PR).

Will revisit if I can just do a single temp-dir for all examples projects.

@@ -0,0 +1 @@
exec devbox run run_test
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will we be adding specific testscripts for other examples? If the testscript is just running devbox run run_test why not do that directly in a regular test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The motivation is ensuring isolation is better...

@savil savil force-pushed the savil/examples-testscript-prototype branch from 104c240 to cfe20ee Compare March 23, 2023 17:46
@savil savil merged commit 55da0d4 into main Mar 23, 2023
@savil savil deleted the savil/examples-testscript-prototype branch March 23, 2023 18:14
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.

2 participants