Skip to content

Add explicit assertions on withArg in the unit tests#1227

Merged
BurningAXE merged 1 commit into
mainfrom
fix-withArg-tests
Jul 1, 2025
Merged

Add explicit assertions on withArg in the unit tests#1227
BurningAXE merged 1 commit into
mainfrom
fix-withArg-tests

Conversation

@BurningAXE
Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE commented Jun 16, 2025

JIRA ticket
Will be released in: 2025.2.0

Root cause analysis (for bugfixes only)

Without an assert, a withArg does nothing*! Therefore we had lots of tests that checked nothing and yes, some of them were wrong.

*The only exception to that is if you specify the type in generic braces <>

Notable changes

  • Replaces withArgs that didn't have effect with match{}
  • Modified two tests in EventDownSyncTaskTest that were no longer correct

Testing guidance

  • Describe how the reviewers can verify that issue is fixed

Additional work checklist

  • Effect on other features and security has been considered
  • Design document marked as "In development" (if applicable)
  • External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
  • Test cases in Testiny are up to date (or ticket created)
  • Other teams notified about the changes (if applicable)

@cla-bot cla-bot Bot added the ... label Jun 16, 2025
@BurningAXE BurningAXE marked this pull request as ready for review June 16, 2025 14:56
@BurningAXE BurningAXE requested review from a team, TristramN, alex-vt, alexandr-simprints, luhmirin-s, meladRaouf and ybourgery and removed request for a team June 16, 2025 14:56
Copy link
Copy Markdown
Contributor

@luhmirin-s luhmirin-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

persistentLogger.log(any(), any(), any(), any())
}
verify { resultMapper.invoke(withArg { it is ActionResponse.EnrolActionResponse }) }
verify { resultMapper.invoke(withArg { assert(it is ActionResponse.EnrolActionResponse) }) }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be using Truth.assertThat() everywhere to be consistent. Or you could also replace most of the instances of withArg with match.

@sonarqubecloud
Copy link
Copy Markdown

@BurningAXE BurningAXE merged commit 0d8e790 into main Jul 1, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants