Skip to content

Commit

Permalink
Assertion function and TestLogger (temporalio#7411)
Browse files Browse the repository at this point in the history
## What changed?
<!-- Describe what has changed in this PR -->

- Added a new function `softassert.That` that logs an error if the
provided condition is false
- Added `testlogger.Testlogger` (copied and slightly modified from our
internal repository) to functional tests

## Why?
<!-- Tell your future self why have you made these changes -->

To allow developers to verify pre- and post-conditions in their code.

**Why not panic?** Maybe in the future. For now, we're happy with
finding these failed assertions in functional tests.

**What about nightlies/long-haul tests?** I've filed a ticket to detect
a failed assertion there and surface them as an alert.

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->

Planted a failing assertion.

**The error log:**
```
logger.go:146: 2025-03-04T20:50:08.578Z	ERROR	log/zap_logger.go:154	Unexpected Error log encountered: failed assertion: wake called twice	{"host": "127.0.0.1:35427", "component": "matching-engine", "wf-task-queue-name": "/_sys/temporal-sys-processor-parent-close-policy/1", "wf-task-queue-type": "Workflow", "wf-namespace": "temporal-system", "worker-build-id": "_unversioned_", "failed-assertion": true, "logging-call-at": "/home/runner/work/temporal/temporal/common/log/with_logger.go:72"}
        go.temporal.io/server/common/log.(*zapLogger).Error
        	/home/runner/work/temporal/temporal/common/log/zap_logger.go:154
        go.temporal.io/server/common/testing/testlogger.(*TestLogger).Error
        	/home/runner/work/temporal/temporal/common/testing/testlogger/testlogger.go:359
        go.temporal.io/server/common/log.(*withLogger).Error
        	/home/runner/work/temporal/temporal/common/log/with_logger.go:72
        go.temporal.io/server/common/log.(*withLogger).Error
        	/home/runner/work/temporal/temporal/common/log/with_logger.go:72
        go.temporal.io/server/common/log.(*withLogger).Error
        	/home/runner/work/temporal/temporal/common/log/with_logger.go:72
        go.temporal.io/server/common/log.(*withLogger).Error
        	/home/runner/work/temporal/temporal/common/log/with_logger.go:72
        go.temporal.io/server/common/softassert.That
        	/home/runner/work/temporal/temporal/common/softassert/softassert.go:50
        go.temporal.io/server/service/matching.(*waitableMatchResult).wake
        	/home/runner/work/temporal/temporal/service/matching/matcher_data.go:560
```


https://github.com/temporalio/temporal/actions/runs/13662479940/job/38196953773?pr=7411#step:9:1638

**The test result:**
```
=== Failed
=== FAIL: tests TestPriorityFairnessSuite/TestPriority_Activity_Basic (0.00s)
    functional_test_base.go:301: Running TestPriorityFairnessSuite/TestPriority_Activity_Basic in test shard 2/3
    functional_test_base.go:373: 
        	Error Trace:	/home/runner/work/temporal/temporal/tests/testcore/functional_test_base.go:373
        	            				/home/runner/work/temporal/temporal/tests/testcore/functional_test_base.go:272
        	            				/home/runner/work/temporal/temporal/tests/testcore/functional_test_base.go:254
        	            				/home/runner/work/temporal/temporal/tests/testcore/functional_test_sdk_suite.go:97
        	            				/home/runner/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:192
        	Error:      	Failing test as unexpected error logs were found.
        	            	Look for 'Unexpected Error log encountered'.
        	Test:       	TestPriorityFairnessSuite/TestPriority_Activity_Basic
```

https://github.com/temporalio/temporal/actions/runs/13662479940/job/38196953773?pr=7411#step:9:1053


## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->

## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->

## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->

---------

Co-authored-by: David Reiss <[email protected]>
Co-authored-by: David Reiss <[email protected]>
Co-authored-by: Tim Deeb-Swihart <[email protected]>
  • Loading branch information
4 people authored Mar 5, 2025
1 parent ec26c25 commit d866361
Show file tree
Hide file tree
Showing 10 changed files with 605 additions and 2 deletions.
3 changes: 3 additions & 0 deletions common/log/tag/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,9 @@ func WorkerComponent(v interface{}) ZapTag {
return NewStringTag("worker-component", fmt.Sprintf("%T", v))
}

// FailedAssertion is a tag for marking a message as a failed assertion.
var FailedAssertion = NewBoolTag("failed-assertion", true)

// history engine shard

// ShardID returns tag for ShardID
Expand Down
48 changes: 48 additions & 0 deletions common/softassert/softassert.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// The MIT License
//
// Copyright (c) 2025 Temporal Technologies Inc. All rights reserved.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package softassert

import (
"go.temporal.io/server/common/log"
"go.temporal.io/server/common/log/tag"
)

// That performs a soft assertion by logging an error if the given condition is false.
// It is meant to indicate a condition that are always expected to be true.
// Returns true if the condition is met, otherwise false.
//
// Example:
// softassert.That(logger, object.state == "ready", "object is not ready")
//
// Best practices:
// - Use it to check for programming errors and invariants.
// - Use it to communicate assumptions about the code.
// - Use it to abort or recover from an unexpected state.
// - Never use it as a substitute for regular error handling, validation, or control flow.
func That(logger log.Logger, condition bool, msg string) bool {
if !condition {
// By using the same prefix for all assertions, they can be reliably found in logs.
logger.Error("failed assertion: "+msg, tag.FailedAssertion)
}
return condition
}
Loading

0 comments on commit d866361

Please sign in to comment.