Merge pull request #232 from buildkite/tet-448-add-skipped-tests-to-t…

…he-buildkite-test-client-report

Add skipped tests to report

internal/plan/type.go

@@ -25,8 +25,9 @@ type Task struct {
 
 // TestPlan represents the entire test plan.
 type TestPlan struct {
-	Experiment string           `json:"experiment"`
-	Tasks      map[string]*Task `json:"tasks"`
-	Fallback   bool
-	MutedTests []TestCase `json:"muted_tests,omitempty"`
+	Experiment   string           `json:"experiment"`
+	Tasks        map[string]*Task `json:"tasks"`
+	Fallback     bool
+	MutedTests   []TestCase `json:"muted_tests,omitempty"`
+	SkippedTests []TestCase `json:"skipped_tests,omitempty"`
 }

internal/runner/discover_test.go

@@ -84,11 +84,13 @@ func TestDiscoverTestFiles_ExcludeNodeModules(t *testing.T) {
 		"testdata/jest/failure.spec.js",
 		"testdata/jest/jest.config.js",
 		"testdata/jest/runtimeError.spec.js",
+		"testdata/jest/skipped.spec.js",
 		"testdata/jest/spells/expelliarmus.spec.js",
 		"testdata/playwright/playwright.config.js",
 		"testdata/playwright/tests/error.spec.js",
 		"testdata/playwright/tests/example.spec.js",
 		"testdata/playwright/tests/failed.spec.js",
+		"testdata/playwright/tests/skipped.spec.js",
 	}
 
 	if diff := cmp.Diff(got, want); diff != "" {

internal/runner/jest.go

@@ -106,6 +106,10 @@ func (j Jest) Run(result *RunResult, testCases []plan.TestCase, retry bool) erro
 				status = TestStatusFailed
 			case "passed":
 				status = TestStatusPassed
+			case "pending":
+				status = TestStatusSkipped
+			case "todo":
+				status = TestStatusSkipped
 			}
 
 			// The scope and name has to match with the scope generated by Buildkite test collector.
@@ -115,6 +119,7 @@ func (j Jest) Run(result *RunResult, testCases []plan.TestCase, retry bool) erro
 				Name:  example.Title,
 				Scope: strings.Join(example.AncestorTitles, " "),
 			}
+
 			result.RecordTestResult(testCase, status)
 		}
 	}

internal/runner/jest_test.go

@@ -150,6 +150,43 @@ func TestJestRun_TestFailed(t *testing.T) {
 	}
 }
 
+func TestJestRun_TestSkipped(t *testing.T) {
+	changeCwd(t, "./testdata/jest")
+
+	jest := NewJest(RunnerConfig{
+		TestCommand: "jest --json --outputFile {{resultPath}}",
+		ResultPath:  "jest.json",
+	})
+
+	t.Cleanup(func() {
+		os.Remove(jest.ResultPath)
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./testdata/jest/skipped.spec.js"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := jest.Run(result, testCases, false)
+
+	if err != nil {
+		t.Errorf("Jest.Run(%q) error = %v", testCases, err)
+	}
+
+	if result.Status() != RunStatusPassed {
+		t.Errorf("Jest.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusPassed)
+	}
+
+	test := result.tests["this will be skipped/for sure"]
+	if test.Status != TestStatusSkipped {
+		t.Errorf("Jest.Run(%q) test.Status = %v, want %v", testCases, test.Status, TestStatusSkipped)
+	}
+
+	todoTest := result.tests["this will be skipped/todo yeah"]
+	if todoTest.Status != TestStatusSkipped {
+		t.Errorf("Jest.Run(%q) todoTest.Status = %v, want %v", testCases, todoTest.Status, TestStatusSkipped)
+	}
+}
+
 func TestJestRun_RuntimeError(t *testing.T) {
 	changeCwd(t, "./testdata/jest")
 
@@ -389,6 +426,7 @@ func TestJestGetFiles(t *testing.T) {
 	want := []string{
 		"failure.spec.js",
 		"runtimeError.spec.js",
+		"skipped.spec.js",
 		"spells/expelliarmus.spec.js",
 	}
 

internal/runner/playwright.go

@@ -84,10 +84,12 @@ func (p Playwright) getTestResultsFromSuite(suite PlaywrightReportSuite, suiteNa
 	for _, spec := range suite.Specs {
 		projectName := spec.Tests[0].ProjectName
 		var status TestStatus
-		if spec.Ok {
-			status = TestStatusPassed
-		} else {
+		if !spec.Ok {
 			status = TestStatusFailed
+		} else if spec.Tests[0].Status == "skipped" {
+			status = TestStatusSkipped
+		} else {
+			status = TestStatusPassed
 		}
 
 		testResults = append(testResults, TestResult{
@@ -166,6 +168,7 @@ func (p Playwright) GetExamples(files []string) ([]plan.TestCase, error) {
 
 type PlaywrightTest struct {
 	ProjectName string
+	Status      string
 }
 
 type PlaywrightSpec struct {

internal/runner/playwright_test.go

@@ -68,6 +68,16 @@ func TestPlaywrightRun_TestFailed(t *testing.T) {
 			Path:  "failed.spec.js:5",
 			Name:  "failed",
 		},
+		{
+			Scope: " chromium failed.spec.js timed out",
+			Path:  "failed.spec.js:14",
+			Name:  "timed out",
+		},
+		{
+			Scope: " firefox failed.spec.js timed out",
+			Path:  "failed.spec.js:14",
+			Name:  "timed out",
+		},
 	}
 
 	if err != nil {
@@ -92,6 +102,34 @@ func TestPlaywrightRun_TestFailed(t *testing.T) {
 	}
 }
 
+func TestPlaywrightRun_TestSkipped(t *testing.T) {
+	changeCwd(t, "./testdata/playwright")
+
+	playwright := NewPlaywright(RunnerConfig{
+		TestCommand: "yarn run playwright test",
+		ResultPath:  "test-results/results.json",
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./testdata/playwright/tests/skipped.spec.js"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := playwright.Run(result, testCases, false)
+
+	if err != nil {
+		t.Errorf("Playwright.Run(%q) error = %v", testCases, err)
+	}
+
+	if result.Status() != RunStatusPassed {
+		t.Errorf("Playwright.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusPassed)
+	}
+
+	test := result.tests[" chromium skipped.spec.js it is skipped/it is skipped"]
+	if test.Status != TestStatusSkipped {
+		t.Errorf("Playwright.Run(%q) test.Status = %v, want %v", testCases, test.Status, TestStatusSkipped)
+	}
+}
+
 func TestPlaywrightRun_Error(t *testing.T) {
 	changeCwd(t, "./testdata/playwright")
 
@@ -221,6 +259,7 @@ func TestPlaywrightGetFiles(t *testing.T) {
 		"tests/error.spec.js",
 		"tests/example.spec.js",
 		"tests/failed.spec.js",
+		"tests/skipped.spec.js",
 	}
 
 	if diff := cmp.Diff(got, want); diff != "" {

internal/runner/rspec.go

@@ -112,6 +112,8 @@ func (r Rspec) Run(result *RunResult, testCases []plan.TestCase, retry bool) err
 			status = TestStatusFailed
 		case "passed":
 			status = TestStatusPassed
+		case "pending":
+			status = TestStatusSkipped
 		}
 
 		result.RecordTestResult(mapExampleToTestCase(example), status)

internal/runner/rspec_test.go

@@ -178,6 +178,36 @@ func TestRspecRun_TestFailedWithoutResultFile(t *testing.T) {
 	}
 }
 
+func TestRspecRun_TestSkipped(t *testing.T) {
+	rspec := NewRspec(RunnerConfig{
+		TestCommand: "rspec --format json --out {{resultPath}} --format progress",
+		ResultPath:  "tmp/rspec.json",
+	})
+
+	t.Cleanup(func() {
+		os.Remove(rspec.ResultPath)
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./testdata/rspec/spec/skipped_spec.rb"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := rspec.Run(result, testCases, false)
+
+	if err != nil {
+		t.Errorf("Rspec.Run(%q) error = %v", testCases, err)
+	}
+
+	if result.Status() != RunStatusPassed {
+		t.Errorf("Rspec.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusPassed)
+	}
+
+	test := result.tests["skipped/is skipped"]
+	if test.Status != TestStatusSkipped {
+		t.Errorf("Rspec.Run(%q) test.Status = %v, want %v", testCases, test.Status, TestStatusSkipped)
+	}
+}
+
 func TestRspecRun_ErrorOutsideOfExamples(t *testing.T) {
 	rspec := NewRspec(RunnerConfig{
 		TestCommand: "rspec --format json --out {{resultPath}} --format documentation",

internal/runner/run_result.go

@@ -96,6 +96,17 @@ func (r *RunResult) MutedTests() []TestResult {
 	return mutedTests
 }
 
+func (r *RunResult) SkippedTests() []plan.TestCase {
+	var skippedTests []plan.TestCase
+
+	for _, test := range r.tests {
+		if test.Status == TestStatusSkipped {
+			skippedTests = append(skippedTests, test.TestCase)
+		}
+	}
+	return skippedTests
+}
+
 // Status returns the overall status of the test run.
 // If there is an error, it returns RunStatusError.
 // If there are failed tests, it returns RunStatusFailed.
@@ -127,39 +138,37 @@ type RunStatistics struct {
 	MutedPassed      int
 	MutedFailed      int
 	Failed           int
+	Skipped          int
 }
 
 func (r *RunResult) Statistics() RunStatistics {
-	var passedOnFirstRun, passedOnRetry, mutedPassed, mutedFailed, failed int
+	stat := &RunStatistics{}
 
 	for _, testResult := range r.tests {
 		switch {
 		case testResult.Muted:
 			switch testResult.Status {
 			case TestStatusPassed:
-				mutedPassed++
+				stat.MutedPassed++
 			case TestStatusFailed:
-				mutedFailed++
+				stat.MutedFailed++
 			}
 
 		case testResult.Status == TestStatusPassed:
 			if testResult.ExecutionCount > 1 {
-				passedOnRetry++
+				stat.PassedOnRetry++
 			} else {
-				passedOnFirstRun++
+				stat.PassedOnFirstRun++
 			}
 
 		case testResult.Status == TestStatusFailed:
-			failed++
+			stat.Failed++
+		case testResult.Status == TestStatusSkipped:
+			stat.Skipped++
 		}
 	}
 
-	return RunStatistics{
-		Total:            len(r.tests),
-		PassedOnFirstRun: passedOnFirstRun,
-		PassedOnRetry:    passedOnRetry,
-		MutedPassed:      mutedPassed,
-		MutedFailed:      mutedFailed,
-		Failed:           failed,
-	}
+	stat.Total = len(r.tests)
+
+	return *stat
 }

internal/runner/run_result_test.go

@@ -193,15 +193,19 @@ func TestRunStatistics(t *testing.T) {
 	r.RecordTestResult(plan.TestCase{Scope: "banana", Name: "is yellow"}, TestStatusFailed)
 	r.RecordTestResult(plan.TestCase{Scope: "banana", Name: "is yellow"}, TestStatusFailed) // test failed twice
 
+	// skipped: 1
+	r.RecordTestResult(plan.TestCase{Scope: "orange", Name: "is orange"}, TestStatusSkipped)
+
 	stats := r.Statistics()
 
 	if diff := cmp.Diff(stats, RunStatistics{
-		Total:            6,
+		Total:            7,
 		PassedOnFirstRun: 2,
 		PassedOnRetry:    1,
 		MutedPassed:      1,
 		MutedFailed:      1,
 		Failed:           1,
+		Skipped:          1,
 	}); diff != "" {
 		t.Errorf("Statistics() diff (-got +want):\n%s", diff)
 	}
@@ -210,6 +214,7 @@ func TestRunStatistics(t *testing.T) {
 func TestRunStatus(t *testing.T) {
 	r := NewRunResult([]plan.TestCase{})
 	r.RecordTestResult(plan.TestCase{Scope: "mango", Name: "is sour"}, TestStatusPassed)
+	r.RecordTestResult(plan.TestCase{Scope: "apple", Name: "is red"}, TestStatusSkipped)
 	if r.Status() != RunStatusPassed {
 		t.Errorf("Status() is %s, want %s", r.Status(), RunStatusPassed)
 	}

internal/runner/test_result.go

@@ -7,7 +7,7 @@ type TestStatus string
 const (
 	TestStatusPassed  TestStatus = "passed"
 	TestStatusFailed  TestStatus = "failed"
-	TestStatusPending TestStatus = "pending"
+	TestStatusSkipped TestStatus = "skipped"
 )
 
 // TestResult is a struct to keep track the result of an individual test case.

internal/runner/testdata/jest/skipped.spec.js

@@ -0,0 +1,7 @@
+describe('this will be skipped', () => {
+  xit('for sure', () => {
+    expect(1).toEqual(2)
+  })
+
+  it.todo('todo yeah')
+})

internal/runner/testdata/playwright/tests/failed.spec.js

@@ -10,3 +10,8 @@ test.describe('test group', () => {
 test('it passes', () => {
   expect(true).toBeTruthy();
 });
+
+test('timed out', async () => {
+  test.setTimeout(100);
+  await new Promise(resolve => setTimeout(resolve, 10000));
+})

internal/runner/testdata/playwright/tests/skipped.spec.js

@@ -0,0 +1,6 @@
+import { expect, test } from '@playwright/test';
+
+
+test.skip('it is skipped', () => {
+  expect(true).toBeTruthy();
+});

internal/runner/testdata/rspec/spec/skipped_spec.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+RSpec.describe "skipped" do
+  xit "is skipped" do
+    expect(true).to be false
+  end
+end