Fix/get members 2173 #2174
Fix/get members 2173 #2174
Conversation
evinosheaforward
force-pushed
the
fix/getMembers-2173
branch
from
55b2cf5 to
9fa30f1
Compare
|
Thanks @evinosheaforward! I will take a look today or early in the week. I expect we'll merge soon and ship a patch to v5, and include in v6. |
Cool - sounds great! I'll note that there may be another bug reported in #2173 edit: made a comment on the issue pointing out that the behavior for the |
There was a problem hiding this comment.
Hey @evinosheaforward - thanks again for this great PR! It definitely fixes the "actions in view reflection" part of #2173.
I think the view/volatiles part might be a separate issue. I'm inclined to handle that in a new PR to keep these things focused. I'll try and put together a test case for that one and respond in the original issue.
With that said, I have a few small requests to finish up this PR. I marked them as blocking because I want to get those changes in before merge, but if you don't have the time to address them this week, let me know and I can handle them.
* test: add failing test to confirm changed behavior issue-2173 * fix: correct getMembers actions/flowactions categorization issue-2173 * chore: update if/else formatting in getMembers for posterity issue-2173 * test: add explicit expect for flow action is added to actions in getMembers issue-2173 * test: add flow action testing to reflection - members chained test issue-2173 --------- Co-authored-by: Evin O'Shea <evinoshea@gmail.com>
* test: add failing test to confirm changed behavior issue-2173 * fix: correct getMembers actions/flowactions categorization issue-2173 * chore: update if/else formatting in getMembers for posterity issue-2173 * test: add explicit expect for flow action is added to actions in getMembers issue-2173 * test: add flow action testing to reflection - members chained test issue-2173 --------- Co-authored-by: Evin O'Shea <evinoshea@gmail.com>
What does this PR do and why?
Fix reported issue with actions being reported as views in getMembers:
#2173
issue was bad if/else if causing actions to be returned as views.
The issues
Steps to validate locally
For brute force exploring the differences, I took the existing v5.1.0 reflection tests and ran them vs both 5.1.0 and master and compared the views, properties, volatiles, and actions. This did yield a result of extra views on master (extra view for each action), so I started a git bisect looking for the commit where that changed and found 62e7e8bafe8bf07f4694e429f734bb12323fd8ef as the "bad commit".
Then I updated the tests to fail in this PR.
Finally, the fix commit passes the new reflection tests