All unit tests should pass checks on GitHub before merge but I have tests that must fail

Question

Upon submitting unit tests (or any code) to GitHub, GitHub Actions will trigger a workflow script. During the execution, it's checking if all unit tests pass. basically it's executing go test ./... command and runs all tests there are.

If any of the tests fail I am not able to merge changes. Which makes sense. But should I write all tests in a way they will pass those checks but at the same time return an error?

pseudocode

testFuncForError()
got := actualError
want := wantedError

if got == want -> pass // we got the error

this test will pass even though we got the error, and the checks will pass.

but

if got != want 
fmt.Printf("got %v, expected %v", got, want) // this will fail

Or for example,

package math

import "testing"

type addTest struct {
    arg1, arg2, expected int
}

var addTests = []addTest{
    addTest{2, 3, 5},
    addTest{4, 8, 12},
    addTest{6, 9, 15},
    addTest{3, 10, 20},
    
}


func TestAdd(t *testing.T){

    for _, test := range addTests{
        if output := Add(test.arg1, test.arg2); output != test.expected {
            t.Errorf("Output %q not equal to expected %q", output, test.expected)
        }
    }
}

In this case, the last Case addTest{3, 10, 20}, will return an error and the test will fail, which would mean that the checks will fail as well. How to test for error but make the test pass?

Your question doesn't make sense. Your last test case will fail, yes, because 3 + 10 isn't equal to 20, but then _why is that a test case?!_ That's **not** an expectation you have for the behaviour of the function. — jonrsharpe, Jul 23 '22 at 07:27
@jonrsharpe That makes sense, but, what if I need to cover the cases where the test is expected to fail? `addTest{1, 10, 13}` I'm passing 1 and 10, but expect 13. What's the approach here? — delos777, Jul 23 '22 at 07:36
The approach is: don't write test cases that would fail even if the correct behaviour was implemented. You **don't** expect an add function to return 13 given 1 and 10, so write `addTest{1, 10, 11}` instead. — jonrsharpe, Jul 23 '22 at 07:38
You *can* write a test case for 3 + 5 != 1234 but that provides very little value. Just write the case for 3 + 5 == 8 — luk2302, Jul 23 '22 at 07:40
You've tagged this [tag:tdd], where you _do_ write test cases that are expected to fail. But they're only expected to fail until the implementation catches up with the requirements expressed by the tests, and given that any failing test says the code doesn't meet requirements why would you _want_ to merge at that point? — jonrsharpe, Jul 23 '22 at 07:44
Or maybe you're asking about tests for cases where the _implementation_ should error, in which case the examples in this post are really bad ones but see e.g. https://stackoverflow.com/q/42035104/3001761. — jonrsharpe, Jul 23 '22 at 07:46
Topic starter picked not very relevant example for question, but testing unhappy code execution path is important and often skipped practice. Undefined or inconsistent behavior for edge cases is at least technical debt, if not a more serious issue. — Dmitry Harnitski, Jul 23 '22 at 15:02

score 0 · Answer 1 · answered Jul 23 '22 at 14:52

What you are asking makes lots of sense for typical Go code that returns result and error.

func Add(arg1 int, arg2 int) (int, error){
   ...
}

For such code, we typically want to run a set of test cases for success path as well as a set of test cases for scenarios that expected to fail.

These two batches of tests often have different assertions:

for success, we check that error is nil and compare result with expected value.
for failure, we often ignore the result because it has undefined behavior. Instead, we check that error is not nil and assert its value. Depending on implementation, we can check error type (often we use errors.Is() and errors.As() for that) or we can compare error message with expected message. Validating error messages is very helpful to check that data posted to logs is descriptive, has no duplicated information or secrets. In GO, making a decision based on error text is bad practice, but unit testing is an exception to that rule.

This is how success path test suite can look like:

type addTest struct {
    arg1, arg2, expected int
}

var addTests = []addTest{
    addTest{2, 3, 5},
    addTest{4, 8, 12},
    addTest{6, 9, 15},
    // removed because it is not success scenario
    // addTest{3, 10, 20},
    
}

func TestAdd(t *testing.T){
    for _, test := range addTests{
        actual, err := Add(test.arg1, test.arg2) 
        require.NoError(t, err)
        assert.Equal(t, test.expected, actual)
    }
}

This is a suite for testing not successful scenarios. It includes only one test case, but it can be easily extended with more test.

type addTestError struct {
    arg1, arg2 int, expected string,
}

var addTestsError = []addTestError{
    addTestError{math.MaxInt64, math.MaxInt64, "result is out of int64 range"},
}

func TestAddError(t *testing.T){
    for _, test := range addTestsError{
        _, err := Add(test.arg1, test.arg2) 
        require.Error(t, err)
        assert.Equal(t, test.expected, err.Error())
    }
}

I like using https://github.com/stretchr/testify for assertions, it makes code cleaner.

All unit tests should pass checks on GitHub before merge but I have tests that must fail

1 Answers1