add UtilsTests for Reflections and Math#58
Open
bacchuongdaungo wants to merge 21 commits into
Open
Conversation
lhmcgann
requested changes
Nov 19, 2024
Owner
lhmcgann
left a comment
lhmcgann
left a comment
There was a problem hiding this comment.
Great start! Check code coverage. And if you could edit the formatting a bit (I started it) and add doc-comments (for test methods and methods tested) and in-line comments for anything more complicated, that would be great
…thods tested and in-line comments for anything more complicated
lhmcgann
requested changes
Jan 27, 2025
Owner
lhmcgann
left a comment
lhmcgann
left a comment
There was a problem hiding this comment.
The existing tests are good, but a few main things:
- check code coverage! There are still some fully untested functions, and many functions with only partial coverage. See the report for line-by-line coverage highlighting.
- The doc comments in the test class are great! Just please add doc comments to all the Utils.cs source reflection functions as well!
- You can delete the GetAvailableStimulatorTypes() utils function in Utils.cs, as well as the associated TODO comment, and don't worry about testing that or letting it mess up your coverage report
… doc comments for them, refactored temp interfaces to avoid confusion, uniformed test name style
… doc comments for them, refactored temp interfaces to avoid confusion, uniformed test name style
lhmcgann
changed the title
lhmcgann
requested changes
Feb 10, 2025
Owner
lhmcgann
left a comment
lhmcgann
left a comment
There was a problem hiding this comment.
@bacchuongdaungo Glad the CI/CD fix was easy, and nice using the IFactory we already have. Check code coverage report for the following Reflection functions:
- IsAssignableFromType()
- IsGenericAssignableFrom()
- GetAvailableGenericTypes()
- IsManufacturableProduct()
And make sure there are doc comments /// for all the reflection and math functions
(I'm including the first few functions in the Math region in this PR since you already included tests for them)
- IsAssignableFrom test cases; some FAILing - comments and other cleanup
lhmcgann
force-pushed
the
test/utils-2
branch
from
0ef8703 to
2b429d5
Compare
- Fixed generic assignability by scanning base classes and interfaces, and by checking type parameters. - Updated dummy type hierarchies and test expectations in UtilsTests.cs. - All tests now pass.
…eEnumerableOfStrings, DictOfSetsToString, SelectFromList, ConstructWithUserInputParams, etc.) - Refined reflection and assignability tests - Minor cleanup and documentation updates
• IsGenericAssignableFrom for non-generic test types • AreTypeParametersCompatible with covariant and incompatible types • ReadJSON error path when file is missing • EnumerableToString with empty enumerable • SelectFromList with invalid then valid input • SelectPort (if available) • RequestConstructorParameterValues using a multi-parameter dummy constructor
…ignableFrom`, and `RequestFactoryCreateParamValues`. - Improved existing tests by removing dummy implementations and leveraging actual IDataLimits implementations. - Fixed TypeConverter issues during testing sequences and collection parsing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix wrong-place utils test