-
Notifications
You must be signed in to change notification settings - Fork 386
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
Improving the ToUnit test coverage #1493
Conversation
- ToUnit_FromNonBaseUnit_ReturnsQuantityWithGivenUnit now also tests the non-base to non-base conversions - ToUnit_FromNonBaseUnit_ReturnsQuantityWithGivenUnit is no longer skipped for quantities with a single unit (the test is still generated) - added the ToUnit_FromIQuantity_ReturnsTheExpectedIQuantity test: covering the IQuantity interfaces - added the Convert_GetTypeCode_Returns_Object test (completing the IConvertible interface coverage) - IQuantity tests: removed the redundant UnitSystem tests and updated "wrong unit type" tests, using all quantities
[Theory{(_quantity.Units.Length == 1 ? "(Skip = \"Multiple units required\")" : string.Empty)}] | ||
[Theory] | ||
[MemberData(nameof(UnitTypes))] | ||
public void ToUnit_FromNonBaseUnit_ReturnsQuantityWithGivenUnit({_unitEnumName} unit) | ||
{{ | ||
// See if there is a unit available that is not the base unit, fallback to base unit if it has only a single unit. | ||
var fromUnit = {_quantity.Name}.Units.First(u => u != {_quantity.Name}.BaseUnit); | ||
|
||
var quantity = {_quantity.Name}.From(3.0, fromUnit); | ||
var converted = quantity.ToUnit(unit); | ||
Assert.Equal(converted.Unit, unit); | ||
Assert.All({_quantity.Name}.Units.Where(u => u != {_quantity.Name}.BaseUnit), fromUnit => | ||
{{ | ||
var quantity = {_quantity.Name}.From(3.0, fromUnit); | ||
var converted = quantity.ToUnit(unit); | ||
Assert.Equal(converted.Unit, unit); | ||
}}); | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could also skip the generation for this method for the single-unit quantities, but decided not to bother (tell me if you'd rather not have it at all).
Of course we could also put back the [Skip]
, however I don't think that's very useful- I'm thinking of the [Skip]
as something that should be addressed, and I don't think that having a single unit is something that needs fixing (as long as it's SI
😈 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind much either way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can of course be moved to the generated tests, but given that we're likely going to be removing this part of the interface, decided to just leave them here for now..
In my other project there are a few other tests here, testing for the IAdditiveIdentity
and such, but I didn't think that it makes sense to bring them in without the new interface definition #1461 )
I also noted the coverage:
and now:
|
There are still some |
[Theory{(_quantity.Units.Length == 1 ? "(Skip = \"Multiple units required\")" : string.Empty)}] | ||
[Theory] | ||
[MemberData(nameof(UnitTypes))] | ||
public void ToUnit_FromNonBaseUnit_ReturnsQuantityWithGivenUnit({_unitEnumName} unit) | ||
{{ | ||
// See if there is a unit available that is not the base unit, fallback to base unit if it has only a single unit. | ||
var fromUnit = {_quantity.Name}.Units.First(u => u != {_quantity.Name}.BaseUnit); | ||
|
||
var quantity = {_quantity.Name}.From(3.0, fromUnit); | ||
var converted = quantity.ToUnit(unit); | ||
Assert.Equal(converted.Unit, unit); | ||
Assert.All({_quantity.Name}.Units.Where(u => u != {_quantity.Name}.BaseUnit), fromUnit => | ||
{{ | ||
var quantity = {_quantity.Name}.From(3.0, fromUnit); | ||
var converted = quantity.ToUnit(unit); | ||
Assert.Equal(converted.Unit, unit); | ||
}}); | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind much either way
Nice improvement here 👏 |
Improving the
ToUnit
test coverage:ToUnit_FromNonBaseUnit_ReturnsQuantityWithGivenUnit
now also tests the non-base to non-base conversionsToUnit_FromNonBaseUnit_ReturnsQuantityWithGivenUnit
is no longer skipped for quantities with a single unit (the test is still generated)ToUnit_FromIQuantity_ReturnsTheExpectedIQuantity
test: covering theIQuantity
interfacesConvert_GetTypeCode_Returns_Object
test (completing theIConvertible
interface coverage)IQuantityTests
: removed the redundantUnitSystem
tests and updated "wrong unit type" tests, using all quantities