Skip to content

Commit

Permalink
Introduce approvaltest module (#4112)
Browse files Browse the repository at this point in the history
* Introduce new approvaltest module

We move tests/approvals and tests/scripts/approvals.go intoa new module,
github.com/elastic/apm-server/approvaltest. This new module will be used
by both apm-server (integration tests) and the systemtest module.

We also reduce the API surface area to two functions: ApproveJSON, and
ApproveEventDocs. The ApproveJSON function can be used for approving
arbitrary JSON-encoded values, while ApproveEventDocs is specifically
intended for approving JSON-encoded Elasticsearch event documents.

To enable the API reduction, we introduce beatertest.EncodeEventDoc(s),
which encode events using the libbeat Elasticsearch output. In some
tests we previously used the libbeat JSON codec directly, which led to
@metadata being included in diffs. The output removes @metadata, hence
it no longer features in our approval diffs.

* Update approved files

- Since we now use the Elasticsearch output, rather than directly
  using the libbeat JSON codec, `@metadata` is no longer part of the
  diff, and `@timestamp` is encoded slightly differently.

- For dynamic fields like `profile.id`, we now replace the value
  with a known string ("dynamic") rather than ignoring the field.

* .ci/docker/golang-mage: support multiple modules
  • Loading branch information
axw committed Aug 31, 2020
1 parent acd41cc commit ac65b7b
Show file tree
Hide file tree
Showing 71 changed files with 806 additions and 1,996 deletions.
8 changes: 7 additions & 1 deletion .ci/docker/golang-mage/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ FROM golang:${GO_VERSION}

ENV TOOLS=/tools
WORKDIR $TOOLS
COPY go.mod .

COPY go.mod go.sum ./
COPY approvaltest/go.mod approvaltest/go.sum ./approvaltest/
COPY systemtest/go.mod systemtest/go.sum ./systemtest/

RUN go mod download
RUN cd approvaltest && go mod download
RUN cd systemtest && go mod download

RUN apt-get update -y -qq \
&& apt-get install -y -qq python3 python3-pip python3-venv \
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ $(PYTHON_BIN)/activate: $(MAGE)

.PHONY: $(APPROVALS)
$(APPROVALS):
@go build -o $@ tests/scripts/approvals.go
@go build -o $@ github.com/elastic/apm-server/approvaltest/cmd/check-approvals

##############################################################################
# Release manager.
Expand Down
2 changes: 1 addition & 1 deletion NOTICE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ Contents of "LICENSE":

--------------------------------------------------------------------
Dependency: github.com/google/go-cmp
Version: v0.4.0
Version: v0.5.2
License type (autodetected): BSD-3-Clause
Contents of "LICENSE":

Expand Down
141 changes: 141 additions & 0 deletions approvaltest/approvals.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package approvaltest

import (
"encoding/json"
"fmt"
"os"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tidwall/gjson"
"github.com/tidwall/sjson"
)

const (
// ApprovedSuffix signals a file has been reviewed and approved.
ApprovedSuffix = ".approved.json"

// ReceivedSuffix signals a file has changed and not yet been approved.
ReceivedSuffix = ".received.json"
)

// ApproveEventDocs compares the given event documents with
// the contents of the file in "<name>.approved.json".
//
// Any specified dynamic fields (e.g. @timestamp, observer.id)
// will be replaced with a static string for comparison.
//
// If the events differ, then the test will fail.
func ApproveEventDocs(t testing.TB, name string, eventDocs [][]byte, dynamic ...string) {
t.Helper()

// Rewrite all dynamic fields to have a known value,
// so dynamic fields don't affect diffs.
events := make([]interface{}, len(eventDocs))
for i, doc := range eventDocs {
for _, field := range dynamic {
existing := gjson.GetBytes(doc, field)
if !existing.Exists() {
continue
}

var err error
doc, err = sjson.SetBytes(doc, field, "dynamic")
if err != nil {
t.Fatal(err)
}
}

var event map[string]interface{}
if err := json.Unmarshal(doc, &event); err != nil {
t.Fatal(err)
}
events[i] = event
}

received := map[string]interface{}{"events": events}
approve(t, name, received)
}

// ApproveJSON compares the given JSON-encoded value with the
// contents of the file "<name>.approved.json", by decoding the
// value and passing it to Approve.
//
// If the value differs, then the test will fail.
func ApproveJSON(t testing.TB, name string, encoded []byte) {
t.Helper()

var decoded interface{}
if err := json.Unmarshal(encoded, &decoded); err != nil {
t.Error(err)
}
approve(t, name, decoded)
}

// approve compares the given value with the contents of the file
// "<name>.approved.json".
//
// If the value differs, then the test will fail.
func approve(t testing.TB, name string, received interface{}) {
t.Helper()

var approved interface{}
readApproved(name, &approved)
if diff := cmp.Diff(approved, received); diff != "" {
writeReceived(name, received)
t.Fatalf("%s\n%s\n\n", diff,
"Run `make update check-approvals` to verify the diff",
)
} else {
// Remove an old *.received.json file if it exists.
removeReceived(name)
}
}

func readApproved(name string, approved interface{}) {
path := name + ApprovedSuffix
f, err := os.Open(path)
if os.IsNotExist(err) {
return
} else if err != nil {
panic(err)
}
defer f.Close()
if err := json.NewDecoder(f).Decode(&approved); err != nil {
panic(fmt.Errorf("cannot unmarshal file %q: %w", path, err))
}
}

func removeReceived(name string) {
os.Remove(name + ReceivedSuffix)
}

func writeReceived(name string, received interface{}) {
f, err := os.Create(name + ReceivedSuffix)
if err != nil {
panic(err)
}
defer f.Close()
enc := json.NewEncoder(f)
enc.SetIndent("", " ")
if err := enc.Encode(received); err != nil {
panic(err)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ package main

import (
"bufio"
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/fatih/color"
"github.com/google/go-cmp/cmp"

"github.com/elastic/apm-server/tests/approvals"
"github.com/elastic/apm-server/approvaltest"
)

func main() {
Expand All @@ -35,16 +37,22 @@ func main() {

func approval() int {
cwd, _ := os.Getwd()
receivedFiles := findFiles(cwd, approvals.ReceivedSuffix)
receivedFiles := findFiles(cwd, approvaltest.ReceivedSuffix)

for _, rf := range receivedFiles {
path := strings.Replace(rf, approvals.ReceivedSuffix, "", 1)
_, _, diff, err := approvals.Compare(path)
if err != nil {
af := strings.TrimSuffix(rf, approvaltest.ReceivedSuffix) + approvaltest.ApprovedSuffix

var approved, received interface{}
if err := decodeJSONFile(rf, &received); err != nil {
fmt.Println("Could not create diff ", err)
return 3
}
if err := decodeJSONFile(af, &approved); err != nil && !os.IsNotExist(err) {
fmt.Println("Could not create diff ", err)
return 3
}

diff := cmp.Diff(approved, received)
added := color.New(color.FgBlack, color.BgGreen).SprintFunc()
deleted := color.New(color.FgBlack, color.BgRed).SprintFunc()
scanner := bufio.NewScanner(strings.NewReader(diff))
Expand All @@ -67,7 +75,7 @@ func approval() int {
input, _, _ := reader.ReadRune()
switch input {
case 'y':
approvedPath := strings.Replace(rf, approvals.ReceivedSuffix, approvals.ApprovedSuffix, 1)
approvedPath := strings.Replace(rf, approvaltest.ReceivedSuffix, approvaltest.ApprovedSuffix, 1)
os.Rename(rf, approvedPath)
}
}
Expand All @@ -84,3 +92,15 @@ func findFiles(rootDir string, suffix string) []string {
})
return files
}

func decodeJSONFile(path string, out interface{}) error {
f, err := os.Open(path)
if err != nil {
return err
}
defer f.Close()
if err := json.NewDecoder(f).Decode(&out); err != nil {
return fmt.Errorf("cannot unmarshal file %q: %w", path, err)
}
return nil
}
9 changes: 9 additions & 0 deletions approvaltest/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module github.com/elastic/apm-server/approvaltest

go 1.15

require (
github.com/google/go-cmp v0.5.2
github.com/tidwall/gjson v1.6.0
github.com/tidwall/sjson v1.1.1
)
12 changes: 12 additions & 0 deletions approvaltest/go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/tidwall/gjson v1.6.0 h1:9VEQWz6LLMUsUl6PueE49ir4Ka6CzLymOAZDxpFsTDc=
github.com/tidwall/gjson v1.6.0/go.mod h1:P256ACg0Mn+j1RXIDXoss50DeIABTYK1PULOJHhxOls=
github.com/tidwall/match v1.0.1 h1:PnKP62LPNxHKTwvHHZZzdOAOCtsJTjo6dZLCwpKm5xc=
github.com/tidwall/match v1.0.1/go.mod h1:LujAq0jyVjBy028G1WhWfIzbpQfMO8bBZ6Tyb0+pL9E=
github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
github.com/tidwall/pretty v1.0.1 h1:WE4RBSZ1x6McVVC8S/Md+Qse8YUv6HRObAx6ke00NY8=
github.com/tidwall/pretty v1.0.1/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
github.com/tidwall/sjson v1.1.1 h1:7h1vk049Jnd5EH9NyzNiEuwYW4b5qgreBbqRC19AS3U=
github.com/tidwall/sjson v1.1.1/go.mod h1:yvVuSnpEQv5cYIrO+AT6kw4QVfd5SDZoGIS7/5+fZFs=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
4 changes: 2 additions & 2 deletions beater/api/intake/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"path/filepath"
"testing"

"github.com/elastic/apm-server/approvaltest"
"github.com/elastic/apm-server/beater/api/ratelimit"

"github.com/stretchr/testify/assert"
Expand All @@ -38,7 +39,6 @@ import (
"github.com/elastic/apm-server/beater/request"
"github.com/elastic/apm-server/processor/stream"
"github.com/elastic/apm-server/publish"
"github.com/elastic/apm-server/tests/approvals"
"github.com/elastic/apm-server/tests/loader"
)

Expand Down Expand Up @@ -148,7 +148,7 @@ func TestIntakeHandler(t *testing.T) {
assert.NotNil(t, tc.c.Result.Err)
}
body := tc.w.Body.Bytes()
approvals.AssertApproveResult(t, "test_approved/"+name, body)
approvaltest.ApproveJSON(t, "test_approved/"+name, body)
})
}
}
Expand Down
12 changes: 6 additions & 6 deletions beater/api/mux_config_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/apm-server/approvaltest"
"github.com/elastic/apm-server/beater/api/config/agent"
"github.com/elastic/apm-server/beater/beatertest"
"github.com/elastic/apm-server/beater/config"
"github.com/elastic/apm-server/beater/headers"
"github.com/elastic/apm-server/beater/request"
"github.com/elastic/apm-server/tests/approvals"
)

func TestConfigAgentHandler_AuthorizationMiddleware(t *testing.T) {
Expand All @@ -40,7 +40,7 @@ func TestConfigAgentHandler_AuthorizationMiddleware(t *testing.T) {
rec, err := requestToMuxerWithPattern(cfg, AgentConfigPath)
require.NoError(t, err)
require.Equal(t, http.StatusUnauthorized, rec.Code)
approvals.AssertApproveResult(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
})

t.Run("Authorized", func(t *testing.T) {
Expand All @@ -50,7 +50,7 @@ func TestConfigAgentHandler_AuthorizationMiddleware(t *testing.T) {
rec, err := requestToMuxerWithHeader(cfg, AgentConfigPath, http.MethodGet, h)
require.NoError(t, err)
require.NotEqual(t, http.StatusUnauthorized, rec.Code)
approvals.AssertApproveResult(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
})
}

Expand All @@ -59,15 +59,15 @@ func TestConfigAgentHandler_KillSwitchMiddleware(t *testing.T) {
rec, err := requestToMuxerWithPattern(config.DefaultConfig(), AgentConfigPath)
require.NoError(t, err)
require.Equal(t, http.StatusForbidden, rec.Code)
approvals.AssertApproveResult(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())

})

t.Run("On", func(t *testing.T) {
rec, err := requestToMuxerWithPattern(configEnabledConfigAgent(), AgentConfigPath)
require.NoError(t, err)
require.NotEqual(t, http.StatusForbidden, rec.Code)
approvals.AssertApproveResult(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
})
}

Expand All @@ -78,7 +78,7 @@ func TestConfigAgentHandler_PanicMiddleware(t *testing.T) {
c.Reset(rec, httptest.NewRequest(http.MethodGet, "/", nil))
h(c)
require.Equal(t, http.StatusInternalServerError, rec.StatusCode)
approvals.AssertApproveResult(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
}

func TestConfigAgentHandler_MonitoringMiddleware(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions beater/api/mux_intake_backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/apm-server/approvaltest"
"github.com/elastic/apm-server/beater/api/intake"
"github.com/elastic/apm-server/beater/beatertest"
"github.com/elastic/apm-server/beater/config"
"github.com/elastic/apm-server/beater/headers"
"github.com/elastic/apm-server/beater/request"
"github.com/elastic/apm-server/tests/approvals"
)

func TestIntakeBackendHandler_AuthorizationMiddleware(t *testing.T) {
Expand All @@ -41,7 +41,7 @@ func TestIntakeBackendHandler_AuthorizationMiddleware(t *testing.T) {
require.NoError(t, err)

assert.Equal(t, http.StatusUnauthorized, rec.Code)
approvals.AssertApproveResult(t, approvalPathIntakeBackend(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathIntakeBackend(t.Name()), rec.Body.Bytes())
})

t.Run("Authorized", func(t *testing.T) {
Expand All @@ -52,7 +52,7 @@ func TestIntakeBackendHandler_AuthorizationMiddleware(t *testing.T) {
require.NoError(t, err)

require.NotEqual(t, http.StatusUnauthorized, rec.Code)
approvals.AssertApproveResult(t, approvalPathIntakeBackend(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathIntakeBackend(t.Name()), rec.Body.Bytes())
})
}

Expand All @@ -63,7 +63,7 @@ func TestIntakeBackendHandler_PanicMiddleware(t *testing.T) {
c.Reset(rec, httptest.NewRequest(http.MethodGet, "/", nil))
h(c)
assert.Equal(t, http.StatusInternalServerError, rec.StatusCode)
approvals.AssertApproveResult(t, approvalPathIntakeBackend(t.Name()), rec.Body.Bytes())
approvaltest.ApproveJSON(t, approvalPathIntakeBackend(t.Name()), rec.Body.Bytes())
}

func TestIntakeBackendHandler_MonitoringMiddleware(t *testing.T) {
Expand Down
Loading

0 comments on commit ac65b7b

Please sign in to comment.