Merge "Show warning annotations and add unit tests"
diff --git a/Makefile b/Makefile
index 7a62a56..e8a68e7 100644
--- a/Makefile
+++ b/Makefile
@@ -40,7 +40,7 @@
# Path to the server binary.
server := backend/dist/fidlbolt
-.PHONY: all help frontend backend run format clean
+.PHONY: all help frontend backend test run format clean
all: frontend backend
@@ -51,6 +51,7 @@
@echo " frontend Build the frontend/dist bundle using webpack."
@echo " For more build options: cd frontend && npm run."
@echo " backend Build the $(server) server binary."
+ @echo " test Build and run all tests.
@echo " run Run the fidlbolt server on \$$PORT."
@echo " Set VERBOSE=1 to enable verbose logging."
@echo " Set DEPLOYMENT=1 to use files in ./deployment instead"
@@ -73,6 +74,9 @@
$(server): backend/go.mod backend/*.go
cd backend && go build -o ../$@
+test:
+ cd backend && go test -v
+
run: $(if $(wildcard frontend/dist),,frontend)
run: $(if $(wildcard $(server)),,backend)
ifndef FUCHSIA_DIR
diff --git a/backend/analyze.go b/backend/analyze.go
index dd83525..38f8ef5 100644
--- a/backend/analyze.go
+++ b/backend/analyze.go
@@ -472,7 +472,7 @@
var fidlAnnotationRegexp = regexp.MustCompile(`` +
// Multi-line mode: ^ and $ match begin/end of line OR text.
`(?m)` +
- `^fidlbolt\.fidl:(\d+):(\d+).*?` +
+ `^/.*/fidlbolt\.fidl:(\d+):(\d+).*?` +
`(` +
string(Info) + `|` +
string(Warning) + `|` +
diff --git a/backend/server.go b/backend/server.go
index 18e795e..7df1db3 100644
--- a/backend/server.go
+++ b/backend/server.go
@@ -322,43 +322,47 @@
return response{}, err
}
defer os.RemoveAll(temp.path)
- res, err := s.handleFIDLWithTemp(ctx, r, temp)
+ fidl, err := temp.createFile("fidlbolt.fidl", r.Content)
+ if err != nil {
+ return response{}, err
+ }
+ libs, anns := s.libraryAnalyzer.analyze(fidl, r.Content)
+ res, err := s.handleFIDLWithTemp(ctx, r, temp, fidl, libs)
if err != nil {
return res, err
}
+ res.Annotations = append(res.Annotations, anns...)
// Remove all occurrences of the temp path to avoid exposing it to the user.
// This is mostly needed for error output, but it is also needed e.g. for
// the JSON IR, which includes source paths.
res.Content = strings.ReplaceAll(res.Content, temp.path+"/", "")
res.Content = strings.ReplaceAll(res.Content, temp.path, "")
- if !res.Ok {
- res.Annotations = append(res.Annotations, parseFidlAnnotations(res.Content)...)
- }
return res, nil
}
const jsonDepIndentSize = 4
-func (s *server) handleFIDLWithTemp(ctx context.Context, r *request, temp tempDir) (response, error) {
- fidl, err := temp.createFile("fidlbolt.fidl", r.Content)
- if err != nil {
- return response{}, err
- }
-
- // Run fidlc, automatically appending the correct --files arguments and
- // producing info annotations for the library declaration and imports.
+func (s *server) handleFIDLWithTemp(ctx context.Context, r *request, temp tempDir, fidl string, libs libraryMap) (response, error) {
+ // Run fidlc, automatically appending the correct --files arguments.
fidlc := func(args ...string) (response, error) {
- libs, anns := s.libraryAnalyzer.analyze(fidl, r.Content)
fileArgs, err := libs.fidlcFileArgs()
if err != nil {
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
- return response{Ok: false, Content: msg, Annotations: anns}, nil
+ return response{Ok: false, Content: msg}, nil
}
- res, err := s.fidlc.run(ctx, append(args, fileArgs...)...)
+ run, err := s.fidlc.run(ctx, append(args, fileArgs...)...)
if err != nil {
return response{}, err
}
- res.Annotations = anns
+ res := response{
+ Ok: run.success,
+ Annotations: parseFidlAnnotations(run.stderr),
+ }
+ if run.success {
+ res.Content = run.stdout
+ } else {
+ res.Content = run.stderr
+ }
return res, nil
}
@@ -371,32 +375,50 @@
switch r.OutputMode {
case FIDL:
+ prog := s.fidlFormat
if r.Options.Lint {
- if res, err := s.fidlLint.run(ctx, fidl); err != nil || !res.Ok {
- return res, err
- }
- return response{Ok: true, Content: "// Lint passed!"}, nil
+ prog = s.fidlLint
}
- return s.fidlFormat.run(ctx, fidl)
+ run, err := prog.run(ctx, fidl)
+ if err != nil {
+ return response{}, err
+ }
+ res := response{
+ Ok: run.success,
+ Annotations: parseFidlAnnotations(run.stderr),
+ }
+ if run.success {
+ if r.Options.Lint {
+ res.Content = "// Lint passed!"
+ } else {
+ res.Content = run.stdout
+ }
+ } else {
+ res.Content = run.stderr
+ }
+ return res, nil
case JSON:
switch r.Options.File {
case "ir":
return fidlc("--json", "/dev/stdout")
case "schema":
- return s.fidlc.run(ctx, "--json-schema")
+ run, err := s.fidlc.runInfallible(ctx, "--json-schema")
+ if err != nil {
+ return response{}, err
+ }
+ return response{Ok: true, Content: run.stdout}, nil
case "deps":
- libs, anns := s.libraryAnalyzer.analyze(fidl, r.Content)
graph, err := libs.dependencyGraph()
if err != nil {
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
- return response{Ok: false, Content: msg, Annotations: anns}, nil
+ return response{Ok: false, Content: msg}, nil
}
b, err := json.MarshalIndent(graph, "", strings.Repeat(" ", jsonDepIndentSize))
if err != nil {
msg := fmt.Sprintf("%s:1:1: error: %s", fidl, err)
- return response{Ok: false, Content: msg, Annotations: anns}, nil
+ return response{Ok: false, Content: msg}, nil
}
- return response{Ok: true, Content: string(b), Annotations: anns}, nil
+ return response{Ok: true, Content: string(b)}, nil
default:
return response{}, fmt.Errorf("invalid file: %q", r.Options.File)
}
@@ -414,30 +436,29 @@
}
jsonIR := fidl + ".json"
- fidlcRes, err := fidlc("--json", jsonIR)
+ res, err := fidlc("--json", jsonIR)
if err != nil {
return response{}, err
}
- if !fidlcRes.Ok {
- return fidlcRes, nil
+ if !res.Ok {
+ return res, nil
}
- res, err := s.handleFIDLWithIR(ctx, r, temp, jsonIR)
+ res.Content, err = s.handleFIDLWithIR(ctx, r, temp, jsonIR)
if err != nil {
return response{}, err
}
- res.Annotations = append(res.Annotations, fidlcRes.Annotations...)
return res, nil
}
-func (s *server) handleFIDLWithIR(ctx context.Context, r *request, temp tempDir, jsonIR string) (response, error) {
+func (s *server) handleFIDLWithIR(ctx context.Context, r *request, temp tempDir, jsonIR string) (string, error) {
switch r.OutputMode {
case HLCPP:
- if res, err := s.fidlgenHlcpp.run(ctx,
+ if _, err := s.fidlgenHlcpp.runInfallible(ctx,
"-json", jsonIR,
"-include-base", temp.path,
"-output-base", temp.join("impl"),
- ); err != nil || !res.Ok {
- return res, err
+ ); err != nil {
+ return "", err
}
switch r.Options.File {
case "header":
@@ -445,15 +466,15 @@
case "source":
return temp.readFile("impl.cc")
default:
- return response{}, fmt.Errorf("invalid file: %q", r.Options.File)
+ return "", fmt.Errorf("invalid file: %q", r.Options.File)
}
case LLCPP:
- if res, err := s.fidlgenLlcpp.run(ctx,
+ if _, err := s.fidlgenLlcpp.runInfallible(ctx,
"-json", jsonIR,
"-header", temp.join("impl.h"),
"-source", temp.join("impl.cc"),
- ); err != nil || !res.Ok {
- return res, err
+ ); err != nil {
+ return "", err
}
switch r.Options.File {
case "header":
@@ -461,24 +482,24 @@
case "source":
return temp.readFile("impl.cc")
default:
- return response{}, fmt.Errorf("invalid file: %q", r.Options.File)
+ return "", fmt.Errorf("invalid file: %q", r.Options.File)
}
case Rust:
- if res, err := s.fidlgenRust.run(ctx,
+ if _, err := s.fidlgenRust.runInfallible(ctx,
"-json", jsonIR,
"-output-filename", temp.join("impl.rs"),
"-rustfmt", s.rustfmt.path,
"-rustfmt-config", s.rustfmtToml,
- ); err != nil || !res.Ok {
- return res, err
+ ); err != nil {
+ return "", err
}
return temp.readFile("impl.rs")
case Go:
- if res, err := s.fidlgenGo.run(ctx,
+ if _, err := s.fidlgenGo.runInfallible(ctx,
"-json", jsonIR,
"-output-impl", temp.join("impl.go"),
- ); err != nil || !res.Ok {
- return res, err
+ ); err != nil {
+ return "", err
}
return temp.readFile("impl.go")
case Dart:
@@ -489,17 +510,17 @@
case "test":
name = "test"
default:
- return response{}, fmt.Errorf("invalid file: %q", r.Options.File)
+ return "", fmt.Errorf("invalid file: %q", r.Options.File)
}
- if res, err := s.fidlgenDart.run(ctx,
+ if _, err := s.fidlgenDart.runInfallible(ctx,
"-json", jsonIR,
"-output-"+name, temp.join("impl.dart"),
- ); err != nil || !res.Ok {
- return res, err
+ ); err != nil {
+ return "", err
}
return temp.readFile("impl.dart")
}
- return response{}, fmt.Errorf("invalid output mode: %q", r.OutputMode)
+ return "", fmt.Errorf("invalid output mode: %q", r.OutputMode)
}
func (s *server) handleFIDLText(ctx context.Context, r *request) (response, error) {
diff --git a/backend/system.go b/backend/system.go
index b889396..d887192 100644
--- a/backend/system.go
+++ b/backend/system.go
@@ -39,6 +39,16 @@
return "", fmt.Errorf("cannot find %s (searched in %s)", name, strings.Join(dirs, ", "))
}
+/// runResult stores the result of running a program.
+type runResult struct {
+ // True if the exit code is zero, false otherwise.
+ success bool
+ // Standard output.
+ stdout string
+ // Standard error.
+ stderr string
+}
+
// normalErrorExitCode is the exit code indicating that a program failed in a
// normal or expected way. For example, fidlc exits with this code when it finds
// a syntax error. We conservatively assume this is the only nonzero exit code
@@ -52,32 +62,53 @@
// the future if fidlbolt uses programs that exit with codes besides 0 and 1.
const normalErrorExitCode = 1
-// run runs a program with arguments. If it completes successfully, returns an
-// Ok response containing stdout. If it exits with normalErrorExitCode,
-// returns a not-Ok response containing stderr. If it fails to launch or exits
-// with any other exit code, returns an error. If ctx is cancelled while the
-// program is running, stops the program and returns an error.
-func (p program) run(ctx context.Context, arg ...string) (response, error) {
+// run runs a program with arguments. If it fails to launch or exits with a code
+// other than 0 or normalErrorExitCode, returns an error. If ctx is cancelled
+// while the program is running, stops the program and returns an error.
+func (p program) run(ctx context.Context, arg ...string) (runResult, error) {
cmd := exec.CommandContext(ctx, p.path, arg...)
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
switch err := cmd.Run().(type) {
case nil:
- return response{
- Ok: true,
- Content: stdout.String(),
+ return runResult{
+ success: true,
+ stdout: stdout.String(),
+ stderr: stderr.String(),
}, nil
case *exec.ExitError:
if err.ExitCode() != normalErrorExitCode {
- return response{}, p.augmentError(err, arg, stdout.String(), stderr.String())
+ return runResult{}, p.augmentError(err, arg, stdout.String(), stderr.String())
}
- return response{
- Ok: false,
- Content: stderr.String(),
+ return runResult{
+ success: false,
+ stdout: stdout.String(),
+ stderr: stderr.String(),
}, nil
default:
- return response{}, err
+ return runResult{}, err
+ }
+}
+
+// runInfallible is like run, but for programs that are not expected to fail. It
+// returns an error for any nonzero exit code, including normalErrorExitCode.
+func (p program) runInfallible(ctx context.Context, arg ...string) (runResult, error) {
+ cmd := exec.CommandContext(ctx, p.path, arg...)
+ var stdout, stderr bytes.Buffer
+ cmd.Stdout = &stdout
+ cmd.Stderr = &stderr
+ switch err := cmd.Run().(type) {
+ case nil:
+ return runResult{
+ success: true,
+ stdout: stdout.String(),
+ stderr: stderr.String(),
+ }, nil
+ case *exec.ExitError:
+ return runResult{}, p.augmentError(err, arg, stdout.String(), stderr.String())
+ default:
+ return runResult{}, err
}
}
@@ -128,10 +159,10 @@
}
// readFile creates a response from a file in the temporary directory.
-func (d tempDir) readFile(name string) (response, error) {
+func (d tempDir) readFile(name string) (string, error) {
b, err := ioutil.ReadFile(d.join(name))
if err != nil {
- return response{}, err
+ return "", err
}
- return response{Ok: true, Content: string(b)}, nil
+ return string(b), nil
}
diff --git a/backend/system_test.go b/backend/system_test.go
new file mode 100644
index 0000000..35b7cc6
--- /dev/null
+++ b/backend/system_test.go
@@ -0,0 +1,172 @@
+// Copyright 2020 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// +build linux darwin
+
+package main
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "os"
+ "testing"
+ "time"
+)
+
+func TestTempDir(t *testing.T) {
+ d, err := newTempDir()
+ if err != nil {
+ t.Fatal("failed to create temp dir: ", err)
+ }
+ defer os.RemoveAll(d.path)
+ written := "the contents of foo"
+ filename, err := d.createFile("foo", written)
+ if err != nil {
+ t.Fatal("failed to create temp file: ", err)
+ }
+ expectedFilename := d.join("foo")
+ if filename != expectedFilename {
+ t.Errorf("got %q, want %q", filename, expectedFilename)
+ }
+ read, err := d.readFile("foo")
+ if err != nil {
+ t.Fatal("failed to read temp file: ", err)
+ }
+ if read != written {
+ t.Errorf("got %q, want %q", read, written)
+ }
+}
+
+func TestFindFile(t *testing.T) {
+ d1, err := newTempDir()
+ if err != nil {
+ t.Fatal("failed to create temp dir: ", err)
+ }
+ defer os.RemoveAll(d1.path)
+ d2, err := newTempDir()
+ if err != nil {
+ t.Fatal("failed to create temp dir: ", err)
+ }
+ defer os.RemoveAll(d2.path)
+
+ d1a, err := d1.createFile("a", "stuff")
+ if err != nil {
+ t.Fatal("failed to create temp file: ", err)
+ }
+ d2a, err := d2.createFile("a", "stuff")
+ if err != nil {
+ t.Fatal("failed to create temp file: ", err)
+ }
+ d2b, err := d2.createFile("b", "stuff")
+ if err != nil {
+ t.Fatal("failed to create temp file: ", err)
+ }
+
+ findD1a, err := findFile("a", []string{d1.path, d2.path})
+ if err != nil {
+ t.Fatal("failed to find d1/a: ", err)
+ }
+ if findD1a != d1a {
+ t.Errorf("got %q, want %q", findD1a, d1a)
+ }
+
+ findD2a, err := findFile("a", []string{d2.path, d1.path})
+ if err != nil {
+ t.Fatal("failed to find d2/a: ", err)
+ }
+ if findD2a != d2a {
+ t.Errorf("got %q, want %q", findD2a, d2a)
+ }
+
+ findD2b, err := findFile("b", []string{d1.path, d2.path})
+ if err != nil {
+ t.Fatal("failed to find d2/b: ", err)
+ }
+ if findD2b != d2b {
+ t.Errorf("got %q, want %q", findD2b, d2b)
+ }
+}
+
+func init() {
+ rand.Seed(time.Now().UTC().UnixNano())
+}
+
+var nonexistentPath = fmt.Sprintf("/does_not_exist.%020d", rand.Uint64())
+
+func TestProgramRunSuccess(t *testing.T) {
+ p := program{"/bin/echo"}
+ ctx := context.Background()
+ run, err := p.run(ctx, "hi")
+ if err != nil {
+ t.Fatal("failed to run echo: ", err)
+ }
+ if !run.success {
+ t.Error("echo did not succeed")
+ }
+ stdout := "hi\n"
+ stderr := ""
+ if run.stdout != stdout {
+ t.Errorf("got %q, want %q", run.stdout, stdout)
+ }
+ if run.stderr != stderr {
+ t.Errorf("got %q, want %q", run.stderr, stderr)
+ }
+}
+
+func TestProgramRunFailure(t *testing.T) {
+ p := program{"/bin/cat"}
+ ctx := context.Background()
+ run, err := p.run(ctx, nonexistentPath)
+ if err != nil {
+ t.Fatalf("%s %s: %s", p.path, nonexistentPath, err)
+ }
+ if run.success {
+ t.Errorf("%s %s: run.success: got %v, want false", p.path, nonexistentPath, run.success)
+ }
+ if run.stdout != "" {
+ t.Errorf("%s %s: run.stdout: got %q, want %q", p.path, nonexistentPath, run.stdout, "")
+ }
+ if run.stderr == "" {
+ t.Errorf("%s %s: run.stderr: got %q, want nonempty", p.path, nonexistentPath, run.stderr)
+ }
+}
+
+func TestProgramRunNoLaunch(t *testing.T) {
+ p := program{nonexistentPath}
+ ctx := context.Background()
+ _, err := p.run(ctx)
+ if err == nil {
+ t.Fatalf("%s: expected error", p.path)
+ }
+}
+
+func TestProgramRunInfallibleSuccess(t *testing.T) {
+ p := program{"/bin/echo"}
+ ctx := context.Background()
+ run, err := p.runInfallible(ctx, "hi")
+ if err != nil {
+ t.Fatal("failed to run echo: ", err)
+ }
+ if !run.success {
+ t.Error("echo did not succeed")
+ }
+ stdout := "hi\n"
+ stderr := ""
+ if run.stdout != stdout {
+ t.Errorf("got %q, want %q", run.stdout, stdout)
+ }
+ if run.stderr != stderr {
+ t.Errorf("got %q, want %q", run.stderr, stderr)
+ }
+}
+
+func TestProgramRunInfallibleFailure(t *testing.T) {
+ p := program{"/bin/cat"}
+ ctx := context.Background()
+ _, err := p.runInfallible(ctx, nonexistentPath)
+ if err == nil {
+ t.Fatalf("%s %s: expected error", p.path, nonexistentPath)
+ }
+}
