internal/cli/run: Remove extra app:ready events that were firing too …

…early (#243)

* fix test removing most app:ready events in runner
* publish app:ready when incrementally reloading too

framework/generator/generator_test.go

@@ -2,9 +2,12 @@ package generator_test
 
 import (
 	"context"
+	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/lithammer/dedent"
 	"github.com/livebud/bud/internal/cli/testcli"
 	"github.com/livebud/bud/internal/is"
 	"github.com/livebud/bud/internal/testdir"
@@ -171,3 +174,64 @@ func TestSyntaxError(t *testing.T) {
 	is.Equal(res.Stderr(), ``)
 	is.Equal(res.Stdout(), ``)
 }
+
+func TestChange(t *testing.T) {
+	t.SkipNow()
+	is := is.New(t)
+	ctx := context.Background()
+	dir := t.TempDir()
+	td := testdir.New(dir)
+	td.Files["generator/tailwind/tailwind.go"] = `
+		package tailwind
+		import (
+			"context"
+			"github.com/livebud/bud/package/overlay"
+		)
+		type Generator struct {
+		}
+		func (g *Generator) GenerateDir(ctx context.Context, fsys overlay.F, dir *overlay.Dir) error {
+			dir.GenerateFile("tailwind.css", func(ctx context.Context, fsys overlay.F, file *overlay.File) error {
+				file.Data = []byte("/** tailwind **/")
+				return nil
+			})
+			return nil
+		}
+	`
+	is.NoErr(td.Write(ctx))
+	cli := testcli.New(dir)
+	app, err := cli.Start(ctx, "run")
+	is.NoErr(err)
+	defer app.Close()
+	// Check for tailwind
+	data, err := os.ReadFile(td.Path("bud/internal/generator/tailwind/tailwind.css"))
+	is.NoErr(err)
+	is.Equal(string(data), "/** tailwind **/")
+	// Update generator
+	generatorFile := filepath.Join(dir, "generator", "tailwind", "tailwind.go")
+	is.NoErr(os.WriteFile(generatorFile, []byte(dedent.Dedent(`
+		package tailwind
+		import (
+			"context"
+			"github.com/livebud/bud/package/overlay"
+		)
+		type Generator struct {
+		}
+		func (g *Generator) GenerateDir(ctx context.Context, fsys overlay.F, dir *overlay.Dir) error {
+			dir.GenerateFile("preflight.css", func(ctx context.Context, fsys overlay.F, file *overlay.File) error {
+				file.Data = []byte("/** preflight **/")
+				return nil
+			})
+			return nil
+		}
+	`)), 0644))
+	// Wait for the app to be ready again
+	fmt.Println("waiting for app to be ready again")
+	is.NoErr(app.Ready(ctx))
+	fmt.Println("waited for app to be ready again")
+	// Check for preflight
+	data, err = os.ReadFile(td.Path("bud/internal/generator/tailwind/preflight.css"))
+	is.NoErr(err)
+	is.Equal(string(data), "/** preflight **/")
+	// Tailwind.css should have been removed
+	is.NoErr(td.NotExists("bud/internal/generator/tailwind/tailwind.css"))
+}

internal/cli/run/run.go

@@ -208,19 +208,16 @@ func (a *appServer) Run(ctx context.Context) error {
 		a.log.Debug("run: published event", "event", "app:error")
 		return err
 	}
-	a.bus.Publish("app:ready", nil)
-	a.log.Debug("run: published event", "event", "app:ready")
 	// Watch for changes
 	return watcher.Watch(ctx, a.dir, catchError(a.prompter, func(events []watcher.Event) error {
-		a.log.Debug("run: files changes", "paths", events)
+		a.log.Debug("run: file changes", "paths", events)
 		a.prompter.Reloading(events)
 		if canIncrementallyReload(events) {
 			a.log.Debug("run: incrementally reloading")
 			// Publish the frontend:update event
 			a.bus.Publish("frontend:update", nil)
 			a.log.Debug("run: published event", "event", "frontend:update")
-			// In this case, the app is still in the "ready" state, but this is useful
-			// for tests that write files and wait for the app to be ready.
+			// Publish the app:ready event
 			a.bus.Publish("app:ready", nil)
 			a.log.Debug("run: published event", "event", "app:ready")
 			a.prompter.SuccessReload()
@@ -249,8 +246,6 @@ func (a *appServer) Run(ctx context.Context) error {
 			return err
 		}
 		a.prompter.SuccessReload()
-		a.bus.Publish("app:ready", nil)
-		a.log.Debug("run: published event", "event", "app:ready")
 		a.log.Debug("restarted the process", "in", time.Since(now))
 		process = p
 		return nil

internal/cli/testcli/testcli.go

@@ -306,7 +306,9 @@ func getURL(path string) string {
 // Wait for the application to be ready
 func (c *Client) Ready(ctx context.Context) error {
 	readySub := c.bus.Subscribe("app:ready")
+	defer readySub.Close()
 	errorSub := c.bus.Subscribe("app:error")
+	defer errorSub.Close()
 	for {
 		select {
 		case <-readySub.Wait():

internal/pubsub/pubsub_test.go

@@ -30,3 +30,18 @@ func TestCloseTwice(t *testing.T) {
 	sub.Close()
 	sub.Close()
 }
+
+func TestSubTwice(t *testing.T) {
+	ps := pubsub.New()
+	sub := ps.Subscribe("toast")
+	ps.Publish("toast", nil)
+	<-sub.Wait()
+	sub.Close()
+	sub = ps.Subscribe("toast")
+	select {
+	case <-sub.Wait():
+		t.Fatal("lingering event")
+	default:
+	}
+	sub.Close()
+}