Refactor and test the compiler #93

matthewmueller · 2022-05-28T10:01:09Z

This is a mammoth of a PR that I should have broken up but... ¯\_(ツ)_/¯

The focus of this PR was to:

Add a lot more testing to key pieces of the framework.
Start to simplify and reduce the number of packages with the same name.
Double down on the existing compiler architecture.
Prepare the framework for upcoming features.

User facing changes:

bud run --port renamed to bud run --listen
Exit codes may have changed (to be tested further)

…ller tests

…ning tests

…local/opt/python@3.8/bin:/Users/m/.cargo/bin:/Users/m/.yarn/bin:/Users/m/.config/yarn/global/node_modules/.bin:/Users/m/Library/Python/2.7/bin:/usr/local/opt/coreutils/libexec/gnubin:/Users/m/.cargo/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/opt/X11/bin:/Library/Apple/usr/bin:/Users/m/dev/bin:/Users/m/dev/src/github.com/matthewmueller/depot_tools:/usr/local/sbin:/Users/m/.composer/vendor/bin

…rror locally

matthewmueller

Hopefully not such a big PR in the future!

matthewmueller · 2022-05-29T07:34:39Z

.github/workflows/test.yml

+      # os.Pipe
+      - name: Install bud binary into $PATH
+        run: go install github.com/livebud/bud@latest
+


This will go away soon, we'll change v8client to use os.Pipe and the same technique we're using with TCP listeners to pass the file descriptors through the binaries via ExtraFiles. This is mostly setup now, just needs to be refactored in.

matthewmueller · 2022-05-29T07:35:10Z

contributing/Dockerfile

+RUN git checkout $BUD_VERSION
+RUN make install
+RUN go install .
+RUN bud version


Helped me track down some Ubuntu-only issues in CI!

matthewmueller · 2022-05-29T07:35:53Z

contributing/Readme.md

+
+```sh
+docker build -t bud:latest contributing
+docker run -it --rm -v $(PWD):/bud bud /bin/bash


This allows you to make modifications in a local bud directory and see those changes within the docker container.

matthewmueller · 2022-05-29T07:39:37Z

internal/cli/cli.go

-		cli.Flag("port", "port").String(&cmd.Port).Default("3000")
-		cli.Run(cmd.Run)
+// Run the CLI and wait for the command to finish
+func (c *CLI) Run(ctx context.Context, args ...string) error {


Swapping out the compiler is the meat of this PR.

The previous implementation in internal/bud wasn't tested or setup for testing. Additionally it exposed too many internals as public APIs making some tests pass without actually working top level.

Now there's just one way to build or run a project. Use: err := cli.Run(ctx, args...) and we can focus our energy on making this well-tested, fast, etc.

The CLI itself exposes a lot of optional configuration in its struct for making this package testable.

matthewmueller · 2022-05-29T07:47:40Z

internal/cli/cli.go

+	cli.Args("args").Strings(&c.args).Default(c.args...)
+	cli.Run(fn)
+
+	{ // $ bud create <dir>


The only CLI commands in here should be bud commands that are available when we're not in a bud project. The reason for this is we want to be able to define project commands that can even wrap commands like bud build, so the fewer commands we have outside the project the better.

matthewmueller · 2022-05-29T08:17:49Z

package/log/log.go

@@ -42,7 +42,7 @@ type Flusher interface {
 	Flush()
 }

-type Logger interface {
+type Interface interface {


log.Logger would always default to the standard libary's log package. This fixes that.

matthewmueller · 2022-05-29T08:20:34Z

package/merged/merged.go

+// are also fs.ErrNotExists
+func (*notExists) Unwrap() error {
+	return fs.ErrNotExist
+}


This is a surprisingly important fix.

The error chain was getting lost when looking up a file that didn't exist in any filesystem (this can happen when there's errors).

We now collect and continue the chain when we encounter fs.ErrNotExist errors.

The collected errors themselves also satisfy errors.Is(err, fs.ErrNotExist).

matthewmueller · 2022-05-29T08:22:54Z

runtime/command/run/run.go

-func (c *Command) compileAndStart(ctx context.Context, ln net.Listener) (*exe.Cmd, error) {
-	app, err := c.Project.Compile(ctx, c.Flag)
-	if err != nil {
+func (c *Command) compileAndStart(ctx context.Context, listener socket.Listener) (*exe.Cmd, error) {


Moved most of the bud {run,build,...} logic into the runtime to prep for wrapping built-in commands.

matthewmueller · 2022-05-29T08:23:55Z

runtime/command/run/run.go

 			if err != nil {
 				console.Error(err.Error())
 				return nil
 			}
-			// TODO: host should be dynamic
-			console.Info("Ready on http://127.0.0.1" + c.Port)
+			process = p


^ Fixes a panic where failures would cause the process to be nil, if the watch command exited, process.Close would panic.

matthewmueller · 2022-05-29T08:26:05Z

runtime/generator/controller/controller_test.go

+	`)
+	// Test stdio
+	is.Equal(stdout.String(), "")
+	is.Equal(stderr.String(), "")


Lots of tests needed to be refactored to use the new testcli package.

matthewmueller added 30 commits May 28, 2022 16:57

add initial compiler test

a4fe23a

test CI against a linked V8

4eeac85

wip compiler refactor

a42820c

starting to consolidate

a809cd4

more consolidation

1d17888

wip

4167724

consolidating

cb1f483

working on new test runner

13772d5

finalize testing apis

3e1dc52

migrating tests

31395f1

improve testcli http response handling and started refactoring contro…

c735268

…ller tests

refactor controller, view and public tests

a155e3f

revert error formatting, will do it top-level instead. refactor remai…

eab452f

…ning tests

extract out file descriptor passing. work on remaining tests

333fa39

fix remaining failed tests

1ee44d2

use go to install bud

6f26ec6

fix public test for CI

3d04db9

fix public tests

7571323

add hot client for testing the hot server. trying to reproduce view e…

2f1436c

…rror locally

add custom flag parsing support

10c17ec

add dockerfile for development and fix failing ubuntu tests

3649a90

update timeout

3a6795d

remove unused go modules

63bfe61

delete unused files

7c9ca94

replace matryer/is with internal/is.

9b51c92

fix formatting

9716d66

fix hot reload and scaffolding new root controllers

96ce936

debug panic

c8300d7

fix test

94870e8

matthewmueller added 3 commits May 29, 2022 02:11

debug panic

7dc82cf

add error check and test for panic seen in CI

d3f11a4

Support Go 1.16+

6430b17

matthewmueller force-pushed the compiler4 branch from 87a4dea to 6430b17 Compare May 29, 2022 07:32

matthewmueller commented May 29, 2022

View reviewed changes

matthewmueller merged commit 2dfa05a into main May 29, 2022

matthewmueller deleted the compiler4 branch May 29, 2022 08:30

matthewmueller mentioned this pull request May 29, 2022

Add compiler tests #73

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor and test the compiler #93

Refactor and test the compiler #93

matthewmueller commented May 28, 2022 •

edited

Loading

matthewmueller left a comment

matthewmueller May 29, 2022 •

edited

Loading

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

matthewmueller May 29, 2022

Refactor and test the compiler #93

Refactor and test the compiler #93

Conversation

matthewmueller commented May 28, 2022 • edited Loading

matthewmueller left a comment

Choose a reason for hiding this comment

matthewmueller May 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewmueller commented May 28, 2022 •

edited

Loading

matthewmueller May 29, 2022 •

edited

Loading