v5.x-fromBase-type-param: Add type parameter to fromBase16 #819
Conversation
This file contains 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
updating test that was failing
Adding fromBase16 to the frontend
set HAS_SECRETS on CI env;
…st to cover Array[Byte] use case.
aslesarenko
requested changes
Jul 14, 2022
| val bytes = Base16.decode(arg.value).get | ||
| tpe match { | ||
| case SByteArray => ByteArrayConstant(bytes) | ||
| case other => ??? |
There was a problem hiding this comment.
see updated version
aslesarenko
requested changes
Jul 16, 2022
| // no type specified | ||
| an[InvalidArguments] should be thrownBy comp(""" fromBase16("0e0131") """) | ||
| // declared type != parsed type | ||
| an[InvalidArguments] should be thrownBy comp(""" fromBase16[Int]("0e0131") """) |
There was a problem hiding this comment.
- use assertExceptionThrown to test exceptions more precisely. See how it is used all over the code.
- Since the method should be generic and work for all types.
This mean all types should be tested. See ConstantSerializerSpecification or DataSerializerSpecification for how it may look like. Those specifications are a bit less generic that they could be.
Ideally you need: - a generator of all valid types
- a generator of valid values for any valid type
- use forAll to test comp(...) for any (type, value) pair
There was a problem hiding this comment.
see updated version
Codecov Report
@@ Coverage Diff @@
## develop #819 +/- ##
===========================================
- Coverage 71.51% 71.50% -0.02%
===========================================
Files 246 246
Lines 18740 18769 +29
Branches 560 620 +60
===========================================
+ Hits 13402 13420 +18
- Misses 5338 5349 +11
Help us with your feedback. Take ten seconds to tell us how you rate us. |
jozanek
force-pushed
the
v5.x-fromBase-type-param
branch
from
66650d5 to
9ab6b62
Compare
…erTest and SigmaParserTest.
|
@aslesarenko please review updated version of PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.