Page MenuHomePhabricator

ecore: add simple test for property_string_add.
ClosedPublic

Authored by cedric on Apr 28 2019, 11:05 AM.

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.
cedric created this revision.Apr 28 2019, 11:05 AM
cedric requested review of this revision.Apr 28 2019, 11:05 AM

Could the test also check that the returned string is the expected one? And check also invalid format strings, to make sure nothing breaks in the future? (I am thinking in degenerate strings like unterminated "%{", or invalid names "%{non-existing-name}")

Good point, will do.

cedric updated this revision to Diff 21852.May 1 2019, 6:07 PM

Rebase and improve test to cover more case.

cedric updated this revision to Diff 21899.May 2 2019, 9:42 AM

Rebase.

Should I use ${} instead of %{} ?

I don't think it matters much, unless we have other places inside EFL where we do such substitutions.

We might. Nothing exposed yet in the new API, but for path with eina_vpath we have (:something:) which I would add support for whatever we choose here. I would like something that people are used to and generally expect. which (:something:) is not.

cedric updated this revision to Diff 22091.May 10 2019, 2:44 PM
cedric added a reviewer: raster.

Rebase and update test for the new syntax.

cedric updated this revision to Diff 22424.May 23 2019, 5:33 PM
cedric retitled this revision from ecore: add simple test for property_text_add. to ecore: add simple test for property_string_add..

Rebase and rename.

segfaultxavi requested changes to this revision.May 27 2019, 3:39 AM

Hmmmm... in the docs we say that not_ready and on_error do not accept placeholder tags, but you are using them in the tests. Why?

This revision now requires changes to proceed.May 27 2019, 3:39 AM

Hmmmm... in the docs we say that not_ready and on_error do not accept placeholder tags, but you are using them in the tests. Why?

Actually the documentation says that they do accept placeholder. I think.

segfaultxavi resigned from this revision.May 29 2019, 11:00 AM

You are right, I misread the docs. No further comments, documentation-wise :)

lauromoura accepted this revision.May 29 2019, 3:36 PM
This revision is now accepted and ready to land.May 29 2019, 3:36 PM
This revision was automatically updated to reflect the committed changes.