feat(assertions): 'arrayWith' and 'objectLike' matchers #15195
Conversation
|
|
nija-at
force-pushed
the
nija-at/assertion-matchers
branch
from
cc29908 to
5daf4bd
Compare
nija-at
force-pushed
the
nija-at/assertion-matchers
branch
from
7712413 to
3956026
Compare
| for (; patternIdx < this.pattern.length; patternIdx++) { | ||
| const p = this.pattern[patternIdx]; | ||
| const e = (Match.isMatcher(p) || typeof p === 'object') ? ' ' : ` [${p}] `; | ||
| failures.push({ path: [], message: `Missing element${e}at pattern index ${patternIdx}` }); |
There was a problem hiding this comment.
This touches on a big problem that the current matchers have: if we DIDN'T match, we can not report any useful information any more to the user.
Ideally, we'd print information on the CLOSEST match we found, by some distance metric that we need to invent. Maybe how deep it got into the tree?
There was a problem hiding this comment.
If we don't want to make the decision right here right now, we should leave the option open to add that later.
That means:
- Either report the entire matching tree back with all alternatives tried & all errors encountered, so that the reporting mechanism has all the information it could possibly use to construct the best error message
- Encapsulate the return value into some class type so that the data it tracks and stores is hidden and can be changed and enhanced at any point in time.
There was a problem hiding this comment.
Maybe flat count of errors is good enough as distance function...
There was a problem hiding this comment.
Yes. I struggled with this a little. I couldn't find a simple solution to this. Instead, I opted to use error count as a distance function. We do need to improve on this, but I was hoping to do it a little more incrementally/iteratively.
Is that reasonable?
we should leave the option open to add that later
Why do we need either of these options?
I was going to simply hide the distance judgement within the Match class and return the final result.
There was a problem hiding this comment.
I was going to simply hide the distance judgement within the Match class and return the final result.
Obviously that, however you're incidentally also exposing the data you're using to make that decision, which means the data is going to be hard to change.
BenChaimberg
added
the
pr/do-not-merge
nija-at
force-pushed
the
nija-at/assertion-matchers
branch
from
438aaea to
a5850c6
Compare
BenChaimberg
removed
the
pr/do-not-merge
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Implement a `hasResource()` API that supports partial matchers like `arrayWith()` and `objectLike()`. This covers the gap with similar support available in the existing assert module. Re-using the same implementation while providing an ergonomic jsii API proved challenging. This change simply re-implements these matchers, heavily influenced by existing implementations in the assert module. Additionally, as a test case, the cognito and synthetics modules are migrated to use the assertions module for its unit tests. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Implement a
hasResource()API that supports partial matchers likearrayWith()andobjectLike().This covers the gap with similar support available in the existing assert
module.
Re-using the same implementation while providing an ergonomic jsii API
proved challenging.
This change simply re-implements these matchers, heavily influenced by
existing implementations in the assert module.
Additionally, as a test case, the cognito and synthetics modules are migrated
to use the assertions module for its unit tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license