Page MenuHomePhabricator

efl_ui: add a helper for not tollerating errors in a testsuite
ClosedPublic

Authored by bu5hm4n on May 25 2019, 7:54 AM.

Details

Summary

We now also fail in the focus tests

Depends on D9021

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bu5hm4n created this revision.May 25 2019, 7:54 AM
bu5hm4n requested review of this revision.May 25 2019, 7:54 AM
zmike requested changes to this revision.May 28 2019, 5:52 AM
zmike added inline comments.
src/tests/elementary/suite_helpers.c
19

These variables should not use the same names as the genlist tests they were copied from.

This revision now requires changes to proceed.May 28 2019, 5:52 AM
bu5hm4n requested review of this revision.May 28 2019, 6:52 AM

I see no value in renaming those variables here, both are static, so completly orthogonal files. Additionally, we also have the variable win all over the place...

bu5hm4n updated this revision to Diff 22475.May 28 2019, 6:56 AM
bu5hm4n edited the summary of this revision. (Show Details)

rebase

bu5hm4n updated this revision to Diff 22478.May 28 2019, 8:02 AM

actaully genlists usage also can be replaced by this helper ...

zmike requested changes to this revision.May 28 2019, 8:45 AM

I see no value in renaming those variables here, both are static, so completly orthogonal files. Additionally, we also have the variable win all over the place...

Citing other places in the codebase which may be suboptimal has no relevance to the issue that I've raised in my review. Good variable naming improves the readability of the codebase, and tree does nothing but confuse any reader of this file.

This revision now requires changes to proceed.May 28 2019, 8:45 AM
bu5hm4n requested review of this revision.May 28 2019, 8:50 AM
In D9022#165417, @zmike wrote:

I see no value in renaming those variables here, both are static, so completly orthogonal files. Additionally, we also have the variable win all over the place...

Citing other places in the codebase which may be suboptimal has no relevance to the issue that I've raised in my review. Good variable naming improves the readability of the codebase, and tree does nothing but confuse any reader of this file.

Your review was concerned about the same variable names in two tests, and that is resolved.

zmike requested changes to this revision.May 28 2019, 8:56 AM

That's an interesting interpretation of my review, but it doesn't address the underlying issue in the review--that the tree component of those variable names has no meaning outside the genlist tree tests which they were originally copied from.

This revision now requires changes to proceed.May 28 2019, 8:56 AM
zmike accepted this revision.May 28 2019, 9:01 AM

Thanks!

This revision is now accepted and ready to land.May 28 2019, 9:01 AM
Closed by commit rEFL8721caf787ce: efl_ui: add a helper for not tollerating errors in a testsuite (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>, committed by zmike). · Explain WhyMay 28 2019, 9:02 AM
This revision was automatically updated to reflect the committed changes.
In D9022#165423, @zmike wrote:

That's an interesting interpretation of my review, but it doesn't address the underlying issue in the review--that the tree component of those variable names has no meaning outside the genlist tree tests which they were originally copied from.

The inital review does line out that they should not use the same name. They did not outline explicitly that your concern is that "tree" does not mean anything here. Next time, please write it like that, i did not understand it.