Merge pull request #2590 from target/testing-docs

dev: add testing section and move smoke tests to test/smoke

CONTRIBUTING.md

@@ -35,5 +35,15 @@ Be sure to run `make check` and tests before opening a PR to catch common errors
 ### Backend Change Guidelines
 
 - Use unit tests as a tool to validate complex logic. For [example](./schedule/rule/weekdayfilter_test.go).
-- New functionality should have a behavioral smoketest at a minimum. For [example](./smoketest/simplenotification_test.go). Documentation on our smoketest framework can be found [here](./smoketest/README.md).
+- New functionality should have a behavioral smoke test at a minimum. For [example](./test/smoke/simplenotification_test.go). Documentation on our smoke test framework can be found [here](./test/smoke/README.md).
 - Go code should [follow best practices](https://golang.org/doc/effective_go.html), exported functions/methods should be commented, etc..
+
+## Testing
+
+GoAlert utilizes 3 main types of testing as tools for different purposes:
+
+- Unit tests are used for complicated logic and exhaustive edge-case testing and benchmarking. They live with the code being tested:
+  - For backend code, a `_test.go` version of a file will contain relevant unit tests. More info [here](https://pkg.go.dev/testing)
+  - For UI code, a `.test.ts` version of a file will contain relevant unit tests. More info [here](https://jestjs.io/docs/getting-started).
+- Smoke tests (in `test/smoke`) are used to ensure main functionality and things like behavioral compatibility with future versions & DB migrations. These focus on hard guarantees like deliverability and preserving intent as the application and datastore evolves and changes over time.
+- Integration tests (currently under `web/src/cypress/integration`) are primarily used to validate happy-path flows work end-to-end, and any important/common error scenarios. They are focused on UX and high-level functionality.

Makefile

@@ -143,11 +143,17 @@ pkg/sysapi/sysapi.pb.go: pkg/sysapi/sysapi.proto $(BIN_DIR)/tools/protoc-gen-go
 generate: node_modules pkg/sysapi/sysapi.pb.go pkg/sysapi/sysapi_grpc.pb.go
 	go generate ./...
 
+
+test-all: test-unit test-smoke test-integration
+test-integration: cy-wide-prod-run cy-mobile-prod-run
+test-smoke: smoketest
+test-unit: test
+
 smoketest:
-	(cd smoketest && go test -parallel 10 -timeout 20m)
+	(cd test/smoke && go test -parallel 10 -timeout 20m)
 
 test-migrations: bin/goalert
-	(cd smoketest && go test -run TestMigrations)
+	(cd test/smoke && go test -run TestMigrations)
 
 tools:
 	go get -u golang.org/x/tools/cmd/gorename

devtools/ci/tasks/scripts/build-all.sh

@@ -5,7 +5,7 @@ PREFIX=$1
 start_postgres
 trap "stop_postgres" EXIT
 
-make check test smoketest cy-wide-prod-run cy-mobile-prod-run bin/goalert BUNDLE=1 CI=1 DB_URL=$DB_URL
+make check test-all bin/goalert BUNDLE=1 CI=1 DB_URL=$DB_URL
 ./bin/goalert self-test --offline
 VERSION=$(./bin/goalert version | head -n 1 |awk '{print $2}')
 

devtools/ci/tasks/scripts/test-smoketest.sh

@@ -2,6 +2,6 @@
 set -x
 start_postgres
 
-trap "stop_postgres; tar czf ../debug/debug-$(date +%Y%m%d%H%M%S)-$(git rev-parse HEAD)-smoketest.tgz -C .. goalert/smoketest/smoketest_db_dump" EXIT
+trap "stop_postgres; tar czf ../debug/debug-$(date +%Y%m%d%H%M%S)-$(git rev-parse HEAD)-smoketest.tgz -C .. goalert/test/smoke/smoketest_db_dump" EXIT
 
-make -e smoketest DB_URL=$DB_URL
+make -e test-smoke DB_URL=$DB_URL

devtools/psql-lite/main.go

@@ -5,7 +5,7 @@ import (
 	"flag"
 	"log"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func main() {

docs/development-setup.md

@@ -73,17 +73,17 @@ The Cypress UI should start automatically.
 
 More information about browser tests can be found [here](../web/src/cypress/README.md).
 
-### Running Smoketests
+### Running Smoke Tests
 
 A suite of functional/behavioral tests are maintained for the backend code. These test various APIs and behaviors
 of the GoAlert server component.
 
-Run the full suite with `make smoketest`.
+Run the full suite with `make test-smoke`.
 
-More information about smoketests can be found [here](../smoketest/README.md).
+More information about smoke tests can be found [here](../test/smoke/README.md).
 
 ### Running Unit Tests
 
-All unit tests can be run with `make test`.
+All unit tests can be run with `make test-unit`.
 
 UI Unit tests are found under the directory of the file being tested, with the same file name, appended with `.test.js`. They can be run independently of the Go unit tests with `make jest`. Watch mode can be enabled with `make jest JEST_ARGS=--watch`.

smoketest/addrules_test.go → test/smoke/addrules_test.go

@@ -1,10 +1,10 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 	"time"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 /*

smoketest/alertcleanup_test.go → test/smoke/alertcleanup_test.go

@@ -1,12 +1,11 @@
-package smoketest
+package smoke
 
 import (
-	"testing"
-
 	"encoding/json"
+	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestAlertCleanup verifies that old alerts are purged from the DB

smoketest/alertlog_test.go → test/smoke/alertlog_test.go

@@ -1,12 +1,12 @@
-package smoketest
+package smoke
 
 import (
 	"encoding/json"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func TestAlertLog(t *testing.T) {
@@ -141,7 +141,7 @@ func TestAlertLog(t *testing.T) {
 		EPStep:     true,
 		EPStepUser: true,
 	}, nil, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var msg = l.Alert.RecentEvents.Nodes[0].Message
+		msg := l.Alert.RecentEvents.Nodes[0].Message
 		assert.Contains(t, msg, "Notification sent")
 		h.Twilio(t).Device(h.Phone("1")).ExpectSMS("Alert #1: foo")
 	})
@@ -154,7 +154,7 @@ func TestAlertLog(t *testing.T) {
 		EPStep:     true,
 		EPStepUser: true,
 	}, nil, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var details = l.Alert.RecentEvents.Nodes[0].State.Details
+		details := l.Alert.RecentEvents.Nodes[0].State.Details
 		assert.Contains(t, details, "contact method disabled")
 	})
 
@@ -168,8 +168,8 @@ func TestAlertLog(t *testing.T) {
 	}, func(t *testing.T, h *harness.Harness) {
 		h.Twilio(t).Device(h.Phone("1")).RejectSMS("Alert #1: foo")
 	}, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var msg = l.Alert.RecentEvents.Nodes[0].Message
-		var details = l.Alert.RecentEvents.Nodes[0].State.Details
+		msg := l.Alert.RecentEvents.Nodes[0].Message
+		details := l.Alert.RecentEvents.Nodes[0].State.Details
 		assert.Contains(t, msg, "Notification sent")
 		assert.Equal(t, "failed", details)
 	})
@@ -184,8 +184,8 @@ func TestAlertLog(t *testing.T) {
 	}, func(t *testing.T, h *harness.Harness) {
 		h.Twilio(t).Device(h.Phone("1")).RejectVoice("foo")
 	}, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var msg = l.Alert.RecentEvents.Nodes[0].Message
-		var details = l.Alert.RecentEvents.Nodes[0].State.Details
+		msg := l.Alert.RecentEvents.Nodes[0].Message
+		details := l.Alert.RecentEvents.Nodes[0].State.Details
 		assert.Contains(t, msg, "Notification sent")
 		assert.Equal(t, "failed", details)
 	})
@@ -198,7 +198,7 @@ func TestAlertLog(t *testing.T) {
 		EPStep:     true,
 		EPStepUser: true,
 	}, nil, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var msg = l.Alert.RecentEvents.Nodes[0].Message
+		msg := l.Alert.RecentEvents.Nodes[0].Message
 		assert.Contains(t, msg, "no immediate rule")
 	})
 
@@ -210,7 +210,7 @@ func TestAlertLog(t *testing.T) {
 		EPStep:     true,
 		EPStepUser: false,
 	}, nil, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var details = l.Alert.RecentEvents.Nodes[0].State.Details
+		details := l.Alert.RecentEvents.Nodes[0].State.Details
 		assert.Contains(t, details, "No one was on-call")
 	})
 
@@ -222,7 +222,7 @@ func TestAlertLog(t *testing.T) {
 		EPStep:     false,
 		EPStepUser: false,
 	}, nil, func(t *testing.T, h *harness.Harness, l alertLogs) {
-		var details = l.Alert.RecentEvents.Nodes[0].State.Details
+		details := l.Alert.RecentEvents.Nodes[0].State.Details
 		assert.Contains(t, details, "No escalation policy steps")
 	})
 }

smoketest/calendarsubscription_test.go → test/smoke/calendarsubscription_test.go

@@ -1,4 +1,4 @@
-package smoketest
+package smoke
 
 import (
 	"encoding/json"
@@ -8,7 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func TestCalendarSubscription(t *testing.T) {

smoketest/cmtestdisable_test.go → test/smoke/cmtestdisable_test.go

@@ -1,9 +1,9 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestCMTestDisable checks for a condition where a test message is scheduled but the contact method is immediately disabled.

smoketest/dedupnotifications_test.go → test/smoke/dedupnotifications_test.go

@@ -1,10 +1,10 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 	"time"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestDedupNotifications tests that if a single contact method is
@@ -50,7 +50,7 @@ func TestDedupNotifications(t *testing.T) {
 	h := harness.NewHarness(t, sql, "escalation-policy-step-reorder")
 	defer h.Close()
 
-	//Test that after 3 minutes, only 1 notification is generated
+	// Test that after 3 minutes, only 1 notification is generated
 	h.FastForward(time.Minute * 3)
 
 	h.Twilio(t).Device(h.Phone("1")).ExpectSMS("testing")

smoketest/deleteescalationpolicy_test.go → test/smoke/deleteescalationpolicy_test.go

@@ -1,10 +1,10 @@
-package smoketest
+package smoke
 
 import (
 	"fmt"
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestDeleteEscalationPolicy tests that it is possible to delete an escalation policy

smoketest/deleterotation_test.go → test/smoke/deleterotation_test.go

@@ -1,10 +1,10 @@
-package smoketest
+package smoke
 
 import (
 	"fmt"
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestDeleteRotation tests that it is possible to delete a rotation with participants

smoketest/emailnotification_test.go → test/smoke/emailnotification_test.go

@@ -1,9 +1,9 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestEmailNotification tests that email notifications are delivered.
@@ -45,5 +45,4 @@ func TestEmailNotification(t *testing.T) {
 	defer h.Close()
 
 	h.SMTP().ExpectMessage(h.Email("1"), "testing")
-
 }

smoketest/escalation_test.go → test/smoke/escalation_test.go

@@ -1,10 +1,10 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 	"time"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestEscalation tests that alerts are escalated automatically, per the delay_minutes setting.

smoketest/escalationgap_test.go → test/smoke/escalationgap_test.go

@@ -1,9 +1,9 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestEscalationGap tests that escalation policy step discrepencies are handled.

smoketest/escalationnotification_test.go → test/smoke/escalationnotification_test.go

@@ -1,10 +1,10 @@
-package smoketest
+package smoke
 
 import (
 	"testing"
 	"time"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 // TestEscalationNotification ensures that notification rules

smoketest/genericapi_test.go → test/smoke/genericapi_test.go

@@ -1,4 +1,4 @@
-package smoketest
+package smoke
 
 import (
 	"bytes"
@@ -7,7 +7,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func TestGenericAPI(t *testing.T) {
@@ -73,5 +73,4 @@ func TestGenericAPI(t *testing.T) {
 	resp.Body.Close()
 
 	h.Twilio(t).Device(h.Phone("1")).ExpectSMS("json")
-
 }

smoketest/genericapiclose_test.go → test/smoke/genericapiclose_test.go

@@ -1,4 +1,4 @@
-package smoketest
+package smoke
 
 import (
 	"bytes"
@@ -8,7 +8,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func TestGenericAPIClose(t *testing.T) {
@@ -112,5 +112,4 @@ func TestGenericAPIClose(t *testing.T) {
 
 	h.FastForward(30 * time.Minute)
 	// closed, no SMS
-
 }

smoketest/genericapidedup_test.go → test/smoke/genericapidedup_test.go

@@ -1,12 +1,12 @@
-package smoketest
+package smoke
 
 import (
 	"bytes"
 	"net/http"
 	"net/url"
 	"testing"
 
-	"github.com/target/goalert/smoketest/harness"
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func TestGenericAPIDedup(t *testing.T) {

smoketest/grafana_test.go → test/smoke/grafana_test.go

@@ -1,10 +1,11 @@
-package smoketest
+package smoke
 
 import (
 	"bytes"
-	"github.com/target/goalert/smoketest/harness"
 	"net/http"
 	"testing"
+
+	"github.com/target/goalert/test/smoke/harness"
 )
 
 func TestGrafana(t *testing.T) {