Writing unit tests for core widgets

[kisvegabor]

Hi,

We have added a new test engine (called Unity) to LVGL and planning to improve the coverage. See this README about how to write and run tests.

I've already created a test for the drop-down list as an example and proof of concept.

Writing unit tests for the whole library at once would be a huge work, so let's do it step by step.

As the first step, I suggest adding tests for every core widget.

If you are interested in contributing with writing tests please let us know here and pick a widget. 🙂

• lv_arc.c
• lv_bar.c
• lv_btn.c
• lv_btnmatrix.c
• lv_canvas.c - will be rework, postponed
• lv_checkbox.c
• lv_dropdown.c
• lv_img.c - will be rework, postponed
• lv_label.c
• lv_line.c
• lv_roller.c
• lv_slider.c
• lv_switch.c
• lv_table.c
• lv_textarea.c
• animimg @pablojr
• calendar
• chart - very complex, 100 USD
• colorwheel
• keyboard
• led - very simple, 25 USD
• list - very simple, 25 USD
• meter - complex, 75 USD
• msgbox
• span - very complex, 100 USD
• spinbox
• spinner
• switch
• tabview
• tileview
• win

Other tests:

• config
• font_loader
• txt
• event
• fs
• mem
• msg
• snapshot
• style
• sw render

---

This is a sponsored issue, meaning that if someone implements it he or she gets a payment from the Accumulated donations of LVGL. Learn more HERE.

We can give 50 USD / fully unit testing a widget with >= 90% coverage.

[C47D]

Are you planning to update this test?

lvgl/tests/src/test_cases/test_dropdown.c

Line 50 in 088b2bd

void test_dropdown_set_text_and_symbol(void)

[kisvegabor]

@C47D
Why is it need to be updated?

[C47D]

It doesn't test anything (I'm looking at master), you're asserting 0 is equal to 0.

void test_dropdown_set_text_and_symbol(void)
{
  TEST_ASSERT_EQUAL(0, 0);
}

[kisvegabor]

Oh, it remained there by accident. I've just removed it.

[C47D]

It should be nice to add tests for fixed bugs, first we need to add a test that replicates the bug (this test should fail), then we make the bug fix, then the test should pass.

[kisvegabor]

I agree, it'd be the ideal case. However, IMO before adding very special test cases it'd be better to add some simple tests for the simple features first.

To get started with we can even set up a a very simple goal: have at least a basic "render" test for every widget.
What do you think?

[C47D]

I agree, but what do you mean by render test?

I've been seeing some talks about how to test Qt projects, and I was thinking on using the same approach here. They don't tend to use screen captures.

I was thinking about #2522 when I made the previous comment. What arc properties controls the drawing direction of the arc, we could replicate the user issue with their code snippet and assert the expected propertie values. But I don't know how LVGL do that.

[kisvegabor]

A lot of things can be tested by setting and getting properties. And IMO it should be the main approach when writing tests.

However comparing some screenshots will easily test a lot of things at once. E.g. widget creation, setting something, drawing, styles, position, size, etc.

[C47D]

Did a test for 2522, just to test the new test improvements, I got the value of expected_start_angle after running the test and letting it fail. But I don't understand why the value is 36, am I missing something?

Before the fix, lv_arc_get_angle_end returned 45.

#if LV_BUILD_TEST
#include "../lvgl.h"

#include "unity/unity.h"

void test_bugfix_for_2522(void);

void test_bugfix_for_2522(void)
{
    uint16_t expected_start_angle = 36;
    uint16_t expected_end_angle = 90;
    int16_t expected_value = 40;

    lv_obj_t *arcBlack;
    arcBlack = lv_arc_create(lv_scr_act());

    lv_arc_set_mode(arcBlack, LV_ARC_MODE_REVERSE);

    lv_arc_set_bg_angles(arcBlack, 0, 90);

    /* lv_arc_set_value calls value_update, where the fix is done */
    lv_arc_set_value(arcBlack, expected_value);

    TEST_ASSERT_EQUAL_UINT16(expected_start_angle, lv_arc_get_angle_start(arcBlack));
    TEST_ASSERT_EQUAL_UINT16(expected_end_angle, lv_arc_get_angle_end(arcBlack));
    TEST_ASSERT_EQUAL_INT16(expected_value, lv_arc_get_value(arcBlack));
}

#endif

[C47D]

I find this article very useful to assign names to test, maybe you find it interesting https://www.codurance.com/publications/2014/12/13/naming-test-classes-and-methods

[C47D]

@kisvegabor have you felt the need to add mocking capability to the test suite?

I've worked a bit with a framework named fff and CMock (also from ThrowTheSwitch). We could add asserts to be sure there are some function calls, with specific parameters and mocking return values.

[kisvegabor]

For first I think we can test many things without mocking, but later it'll be surely required to make more isolated and specific tests.

If CMock has all the required features probably it should be added as it works well with Unity.

[C47D]

Do you have any priority list for adding tests? I've seen some interesting bugs in the calendar widget.

Also, one question, when the checkbox state is disabled and the user clicks on it, should it's event handler be called? I guess it shouldn't, but I'm not sure about that.

[vahidajalluian]

@C47D
Have you encountered the same bug as the one I have reported recently for the calendar widget?

[C47D]

Hi, no, but I was wondering if we can replicate it using unit tests.

[kisvegabor]

Do you have any priority list for adding tests?

It's not a strong opinion but I think adding tests to the widgets is a good start because users have direct connection with them. So we test what is directly used, and we can find contributors easier to such tangible topic.

If the question is which widgets should be first, it doesn't really matter. If we get started with it all widgets should have test soon.

[C47D]

@kisvegabor
I think we should update the list of widgets on the first commit. Do you want me to add test for a particular widget next?

[kisvegabor]

I think we should update the list of widgets on the first commit.

At first I added only the core widgets and not the ones from the extra folder. I suggest finishing these first and opening a new issue for the rest.

Do you want me to add test for a particular widget next?

Switch, bar and slider are simple, I think they can be easily tested. Are interested in them?

Probably Label and Textarea are the most complex ones and there are some related misc modules (lv_bidi, lv_txt, lv_txt_ap) that should be also tested. I can work on these.

[C47D]

At first I added only the core widgets and not the ones from the extra folder. I suggest finishing these first and opening a new issue for the rest.

I meant to update the checkboxes, as some of them have test already.

Switch, bar and slider are simple, I think they can be easily tested. Are interested in them?

Sure, I will.

[kisvegabor]

Update. Let me know if I missed something.

[C47D]

Update. Let me know if I missed something.

It's all good, I will add the tests for the widgets you requested on the coming weeks.

I've just noticed that this commit is not applied? 7278089

[kisvegabor]

I've just noticed that this commit is not applied? 7278089

I've added it again here b279f63
Maybe it was overwritten by a force push, sorry for that. 🙁

[kisvegabor]

@some00 Sure! Do you have any idea about how to test it? If it will be very simple (to be fair with other) we give 25 USD for it.

[some00]

@kisvegabor My plan is to take a few screenshots between ticks.

[kisvegabor]

Sorry for the late reply, I'm just back from the Christmas holiday.

My plan is to take a few screenshots between ticks.

It's okay for me, but in this case I think 25 USD would be more appropriate. Are you ok with it?

[some00]

@kisvegabor It wasn't a big deal. If you insist on the bounty, please donate it to any dog shelter or archive.org.

[kisvegabor]

I've just merged the PR.

If you insist on the bounty, please donate it to any dog shelter or archive.org.

To be honest I wouldn't like to establish the practice of I spend others payment. If you wouldn't like to "bother" with 25 USD I understand and you can say "Gabor, keep it and give it to other contributors who do something for LVGL". But I'd be happier if I could give it to you.

[some00]

Gabor, keep it and give it to other contributors who do something for LVGL. I didn't mean any disrespect, those causes are precious to me.

[kisvegabor]

those causes are precious to me.

That's why I'd be happy to give that money to you so that you could donate to causes which are precious to you.
Anyway, let me know if you have changed you mind. ☺️

[raulgotor]

Hello! Is it ok to work on the tests for the meter or span widgets? 🙂

[kisvegabor]

Hi,

meter will be replaced with lv_scale in v9, however working on span would be great!

[raulgotor]

Ok! I'll go straight ahead to span ;)

[kisvegabor]

Great, thank you!

[raulgotor]

Hey @kisvegabor, tests for lv_span.c are ready. So far, I found some issues:

• lv_spangroup_get_lines not returning current lines number (-1 is returned)
• spangroup_set_mode would accept any mode outside valid options (no LV_SPAN_MODE_COUNT or LV_SPAN_MODE_MAX available neither)

Would you like me to push the tests?

[kisvegabor]

Hi @raulgotor,

Yes, please push the tests, and if you have suggestions to fix the mentioned issues it would be also great.

As far as I know the span widget doesn't support \n now. If you could implement it, I would be happy to increase the 100 USD reward to 150. 🙂

[raulgotor]

I think I don't have the right permissions to push. Can you set me as contributor?

For the other topics, I'll look on them and create respective PRs if I find and can fix the issues. Thanks!

[kisvegabor]

I think I don't have the right permissions to push. Can you set me as contributor?

Please check out here how yo can send a Pull request.

For the other topics, I'll look on them and create respective PRs if I find and can fix the issues.

Thank you! 👍

[raulgotor]

Ok, PR created.

On the other hand, regarding the issues:

• lv_spangroup_get_lines issue: there was no real issue, just misunderstood what the API was meant to do. Since lv_spangroup_set_lines(lv_obj_t * obj, int32_t lines) is meant to set the max lines number that span can show when LV_SPAN_MODE_BREAK, I would suggest to rename to lv_spangroup_set_lines_max(lv_obj_t * obj, int32_t lines_max) as well as lv_spangroup_get_lines_max or similar. As a user of the API, first guess was this was returning the current number of lines the text was spreading through.
• spangroup_set_mode would accept any mode outside valid options...
  I tend to do this the following way:

enum _lv_span_mode_t {
     LV_SPAN_MODE_FIXED,     /**< fixed the obj size*/
     LV_SPAN_MODE_EXPAND,    /**< Expand the object size to the text size*/
     LV_SPAN_MODE_BREAK,     /**< Keep width, break the too long lines and expand height*/
     LV_SPAN_MODE_COUNT,     /**< Fence member*/
};

i.e, by adding LV_SPAN_MODE_COUNT to the enum. Then, at the implementation one can do:

void lv_spangroup_set_mode(lv_obj_t * obj, lv_span_mode_t mode)
{
    ...
    if (mode >= LV_SPAN_MODE_COUNT) {
        // Log an error and exit function
        return;
    }
    ...
} 

But then I've seen that at least the functions i've checked on the API allow to pass enum values out of range without any check, so I guess that there is a reasoning for that. I mean, I would be happy to push a fix for this particular function, but it won't follow same pattern as other functions having enums as parameters. WDYT?

[kisvegabor]

There related PR is: #4595

• I agree to rename lv_spangroup_get_lines, but let use the lv_spangroup_get_max_lines name.
• In other places we used the _LV_SPAN_MODE_LAST convention. I absolutely agree to add and use it.

But then I've seen that at least the functions i've checked on the API allow to pass enum values out of range without any check

Do you mean other parts of LVGL (not only span)? If so, it's really possible, but we need to start somewhere. Having it only at least here showcases a good practice. 🙂

[raulgotor]

• Ok, I'll do the renaming of lv_spangroup_get_lines as suggested. Should I do lv_spangroup_set_lines as well?
• I'll stick to current convention and will add bound checks to the functions in this module
• Would you like to do this in different PRs?

[kisvegabor]

Thank you. Please send the fixed to the current PR.

[raulgotor]

Hello @kisvegabor, pushed new changes, fixed the two topics I found. Please have a check.

Regarding the support for \n, I was checking on it, and while I got to get some results, my implementation was buggy and realized that don't have the complete understanding of the current implementation in order to fix that. Maybe it would be worth if I directly talk with @guoweilkd as he was implementing it.

[kisvegabor]

I've just merged the PR, thank you!
Please send an expense at https://opencollective.com/lvgl

Maybe it would be worth if I directly talk with @guoweilkd as he was implementing it.

Okay, hoefully @guoweilkd can take look.

[raulgotor]

Oh I'm glad it went through! Ok I'll do that.

By the way, regarding keyboard and tileview, do they need tests as well, right? I could take a look if you agree once I have some time

[kisvegabor]

By the way, regarding keyboard and tileview, do they need tests as well, right? I could take a look if you agree once I have some time

Yes, it'd be great to have tests for those as well! 🚀

[5z5]

By the way, regarding keyboard and tileview, do they need tests as well, right? I could take a look if you agree once I have some time

Yes, it'd be great to have tests for those as well! 🚀

can i use centos to test the case?

[kisvegabor]

can i use centos to test the case?

It should work. Just follow the guide from here.

[5z5]

can i use centos to test the case?

It should work. Just follow the guide from here.

thank you

[5z5]

can i use centos to test the case?

It should work. Just follow the guide from here.

I want to know the meter widget have been cancel? I can't find it in lvgl docs.

[kisvegabor]

Hi,
It's replaced by the more flexible scale.