Unit Test Assert does not allow to check method calls for idempotence

[PeregrinTooc]

💡 Code pal for ABAP follows the Clean ABAP. If the issue relates to coding style, please submit it here.

Check Name
unit test assert

Actual Behavior
If the same method is called twice in actual and expected, e.g. to test that a factory method returns a singleton, 'illegal assert' is raised

Expected Behavior
For method calls, dispense with the rule, since idempotence is a meaningful test.

[lucasborin]

Unfortunately, Code Pal performs a static check (string-based), and it cannot detect idempotent calls.
What we could implement is exempting if the act&exp ends with get_instance( ). How about it?

[pokrakam]

I think that's too specific, you can also have

cl_abap_unit_assert=>assert_differs( 
  act = get_next( )
  exp = get_next( ) ).

But I also think this is a narrow use case that is easy enough to work around by assigning one of the values to a variable.

[pokrakam]

Actually I just spotted that we're only talking about method calls. In this case it should be sufficient to exclude anything that has a "(" in it, no?

[lucasborin]

Actually I just spotted that we're only talking about method calls. In this case it should be sufficient to exclude anything that has a "(" in it, no?

I am with you guys, it makes sense to disable this rule on these method calls due to this technical limitation. I am unsure about the ( because you could compare a slice of a string in the act vs exp, like:

DATA(memory) = '010101110101'.

cl_abap_unit_assert=>assert_differs( act = memory+2(1)
                                     exp = memory+3(1) ).

[lucasborin]

I was wondering to check the tokens => and -> as usually you will call a cut method. However, local methods won't be supported by this rule.

" Singleton
cl_abap_unit_assert=>assert_equals( act = my_class=>get_instance( )
                                    exp = my_class=>get_instance( ) ).

[pokrakam]

If you're treating it as a string then matching ^[a-z0-9_]+\( should avoid the substring variation.

[PeregrinTooc]

Why not just extend it not to trigger if it ends with ‚( ).‘ so all method calls are exempt? Lucas Borin ***@***.***> schrieb am Mi. 15. Sept. 2021 um 16:03:

…

Unfortunately, Code Pal performs a static check (string-based), and it cannot detect idempotent calls. What we could implement is exempting if the act&exp ends with get_instance( ). How about it? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#460 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AN5H6UCQFHCECVFWYB22XXDUCCRSBANCNFSM5ECCN35A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[PeregrinTooc]

Oh wait, too late for me to really think straight 🙈 parameters might be there 😂 Philipp Rembold ***@***.***> schrieb am Mi. 15. Sept. 2021 um 22:36:

…

Why not just extend it not to trigger if it ends with ‚( ).‘ so all method calls are exempt? Lucas Borin ***@***.***> schrieb am Mi. 15. Sept. 2021 um 16:03: > Unfortunately, Code Pal performs a static check (string-based), and it > cannot detect idempotent calls. > What we could implement is exempting if the act&exp ends with get_instance( > ). How about it? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#460 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AN5H6UCQFHCECVFWYB22XXDUCCRSBANCNFSM5ECCN35A> > . > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > >

[pokrakam]

Yes, on second thoughts it's a little more complex. The expression [a-z_][a-z0-9_]*\( should handle more complex expressions.
It ignores foo && bar+2(1) but matches cls=>foo && obj->bar( ) && moo+2(1)

[lucasborin]

@PeregrinTooc fix available under branch 1.16.0. Feel free to test it. 😃
Btw: Thanks Mike!