Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce the Test.assertEqual function #2571

Merged

Conversation

m-Peter
Copy link
Contributor

@m-Peter m-Peter commented Jun 13, 2023

Closes onflow/cadence-tools#93

Description

This function has built-in support for showing the difference between the expected and the actual value. For example:

Test results: "tests/test_foo_contract.cdc"
- FAIL: testGetIntegerTrait
		Execution failed:
			error: assertion failed: not equal: expected: "Hugee", actual: "Huge"
			  --> 7465737400000000000000000000000000000000000000000000000000000000:27:8
			
- PASS: testAddSpecialNumber
Coverage: 95.7% of statements

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

This function has built-in support for showing the difference
between the expected and the actual value.
@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Merging #2571 (e77dffe) into master (1c6994f) will increase coverage by 0.00%.
The diff coverage is 89.47%.

@@           Coverage Diff           @@
##           master    #2571   +/-   ##
=======================================
  Coverage   78.54%   78.55%           
=======================================
  Files         336      336           
  Lines       77666    77704   +38     
=======================================
+ Hits        61005    61042   +37     
+ Misses      14386    14384    -2     
- Partials     2275     2278    +3     
Flag Coverage Δ
unittests 78.55% <89.47%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
runtime/stdlib/test_contract.go 87.88% <89.47%> (+0.07%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

@turbolent turbolent self-assigned this Jun 16, 2023
@turbolent turbolent merged commit 728de64 into onflow:master Jun 16, 2023
Label: sema.ArgumentLabelNotRequired,
Identifier: "actual",
TypeAnnotation: sema.NewTypeAnnotation(
sema.AnyStructType,
Copy link

@kozlovb kozlovb Jun 16, 2023

Choose a reason for hiding this comment

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

I wonder if equality of two types could be enforced easily.

By that I mean that if types are not equal then the very first test execution is not going to pass anyway. So, its not something that important. On the other hand having comparisons of ints to strings fail at the compile time could've been nice too.

Also, may be there is already a check at the compile time that I've missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

import Test

pub fun test() {
	Test.assertEqual(true, 1)
}

We have such a test, and the assertion error is the following:

assertion failed: not equal: expected: true, actual: 1

But since the assertEqual method should work for any sub-type of AnyStruct, this is the expected behavior.

On the other-hand, for types that do not support equality, e.g resources, we also have a test case:

import Test

pub fun test() {
	let f1 <- create Foo()
	let f2 <- create Foo()
	Test.assertEqual(<-f1, <-f2)
}

pub resource Foo {}

And it fails with:

errs := checker.RequireCheckerErrors(t, err, 2)
assert.IsType(t, &sema.TypeMismatchError{}, errs[0])

Copy link

Choose a reason for hiding this comment

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

Right, cadence is interpreted language, my bad.

Great, that you've covered that in test. "not equal: expected: true, actual: 1" as discussed in PMs may be error message can specify that types differ.

Copy link

Choose a reason for hiding this comment

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

Although, commit is merged and I think this point is "matter of taste"/minor. So, fine for me either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, thanks for pointing it out though 🙌

Copy link
Contributor Author

@m-Peter m-Peter left a comment

Choose a reason for hiding this comment

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

All good!

@m-Peter m-Peter deleted the add-the-assert-equal-test-function branch June 21, 2023 13:36
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.

[Test] Improve test failure output
3 participants