AF-2799: Remove mirrors from tests #219
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
Codecov Report
@@ Coverage Diff @@
## master #219 +/- ##
=========================================
- Coverage 95.82% 90.1% -5.71%
=========================================
Files 35 35
Lines 1767 1767
=========================================
- Hits 1693 1592 -101
- Misses 74 175 +101 |
| test('cannot set / read values that are not its prop map', () { | ||
| var instance = instanceBuilder(); | ||
| expect(() {instance['notThere'];}, | ||
| throwsA(hasToStringValue(contains('Map does not contain this key')))); | ||
| }); | ||
| for (var propKey in keys) { | ||
| test('prop: $propKey can have its value set / read', () { |
There was a problem hiding this comment.
These tests were only intended to verify that the code generation for implementing concrete getters/setters was working properly, and that these props classes had a prop key namespace of '', (except for the case of aria props, which are prefixed with aria-). Therefore, these tests have been simplified here to just test on a few of the props on a mixin.
| 'react.SyntheticEvent', | ||
| 'react.SyntheticMouseEvent', | ||
| ], override: 'mockito') | ||
| import 'dart:mirrors'; |
There was a problem hiding this comment.
@greglittlefield-wf were these needed? afaik, the only usage of mirrors within mockito was spy(), which I didn't find any usages of in over_react's tests.
There was a problem hiding this comment.
Mm, looks like these were added under the false assumption that Mock classes used mirrors.
Away with them! 😄
|
+10 CI does indeed pass |
There was a problem hiding this comment.
Looks good, just one nit. It would be good to try out Sorry – forgot all of these mirrors usages were outside of pub build on wdesk using this branch just to ensure there aren't any other hidden mirrors issues that might crop up during dart2js compilation.lib/.
| @@ -83,12 +82,12 @@ main() { | |||
| if (expectedTagName == 'missingGlyph') expectedTagName = 'missing-glyph'; | |||
| if (expectedTagName.startsWith(new RegExp('svg.'))) expectedTagName = expectedTagName.substring(3); | |||
|
|
|||
| test('Dom.$name generates the correct type', () { | |||
| DomProps builder = domClassMirror.invoke(element, []).reflectee; | |||
| test('${method.toString()} generates the correct type', () { | |||
There was a problem hiding this comment.
Using .toString() inside a string interpolation is redundant:
| test('${method.toString()} generates the correct type', () { | |
| test('$method generates the correct type', () { |
|
@Workiva/release-management-pp |
Ultimate problem:
Usages of dart mirrors go away in dart 2
How it was fixed:
Remove usages of dart mirrors.
Testing suggestions:
[ ] CI Passes
Potential areas of regression:
Test coverage