Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for test hierarchies #4135

Closed
jerabaul29 opened this issue Dec 8, 2021 · 11 comments
Closed

Added support for test hierarchies #4135

jerabaul29 opened this issue Dec 8, 2021 · 11 comments

Comments

@jerabaul29
Copy link

jerabaul29 commented Dec 8, 2021

I finally got the time to get into the details of how to use the unit testing part of PlatformIO. First, many thanks for a super nice feature :) .

A few comments below. Sorry for the length, this is really because I love this feature and would love to see it be even better if it can be that I write something so long, so please take it as compliment not criticism :) .

the "what is included for building tests" question

It was quite confusing to me at first that plaformio unit testing does not use any of the files / headers in src and include. After a bit of confusion, I found the platformio web page documenting that unit testing by default only uses the lib content, and that a (discouraged) platformio.ini flag is needed to also use what comes from the src. This is the kind of thing that I was very confused about - I know "a posteriori" that I should have read the documentation in more details, but just wondering if this information may be added to the readmes of the src and include folders of each project :) .

the "tests folders need to be at the root of test" question

Another thing I was a bit confused about is that the tests always need to live in the "root" of the test folder. What I mean is that it is ok to have a structure:

- test
   | - test_embedded_test_1
   |     |- main.cpp
   |     |- util.cpp
   | - test_embedded_test_2
   |     |- main.cpp
   |     |- another_util.cpp

but that the following structure fails to run tests, complains about multiple setup and loop functions etc, and that was confusing for me at first:

- test
   | embedded
         | - test_1
         |     |- main.cpp
         |     |- util.cpp
         | - test_2
         |     |- main.cpp
         |     |- another_util.cpp

Having a possibility to have a bit of hierarchy would maybe be convenient though - for grouping stuff by category etc. Would it maybe be an option to allow the user to put a .test_hierarchy file or similar in folders that are not the root of a test case, but an "organization level" for a set of tests? Let me know if this is unclear. Maybe there is a better strategy, too.

the "bad tests setup" question

I first set up my tests wrongly, and then the test suite was not failing, just freezing. This was also very confusing. The issue was: tests need to be within a "RUN_TEST()" command and not just "natively", i.e.:

  • this works:
void setup() {
    UNITY_BEGIN();

    RUN_TEST(some_test);

    UNITY_END();
}
  • this does not work (freeze), without error messages, which is confusing to me:
    UNITY_BEGIN();

    some_test;

    UNITY_END();
}

This is the kind of stupid mistake that happens, and is was very confusing to me that stuff just froze, without any meaningful error message or else... Wondering if there would be a way to catch this at compile time.

the caching question

Would it be possible to perform caching of all tests with all their dependencies etc individually and in "parallel" of the src, so that it is fast to run and re run tests and project building / upload back and forth? I think I observed the following behavior:

  • if I build / upload several times in a row this is very fast, thanks to src build caching
  • if I test several times in a row this is very fast, thanks to test build caching
  • if I build then test then build, the second build is slow and need to recompile everything
  • if I test then build then test, the second test is slow and need to recompile everything

Can you confirm that this is well the behavior that is happening at the moment and that I am not missing something? :)

What about / would it be possible to set things so that, for example, both the src build and test(s) build(s) get cached "side by side" of each other, so that one can have fast builds in all cases even switching back and forth? I (and most people today :) ) have plenty of disk space, so I would not mind if the platformio build cache for a project is real 1 + N platformio build cache folders, with 1 src cache and N individual test caches (1 for each test). Of course if there can be an even better strategy that would be great too :) .

@jcw
Copy link

jcw commented Dec 12, 2021

  • "bad tests setup": this is due to the Unity test framework, not much PIO can do about this, I think
  • "the caching question": in test mode, code is built very differently (with #define UNIT_TEST set as one example), so any code in lib/ which is linked into test or run mode must be compiled in the corresponding mode - again, probably not much PIO can do probably, other than a major redesign to build the run & test apps in separate build areas

PS. There may be a way to work around that second point (untested). Try this: create a 2nd [env:name2] entry with extends = <name-of-first-entry> in it, then use pio test and pio run -e name2 when you alternate between the two.

@Adrian-Samoticha
Copy link

Regarding the "what is included for building tests" question:

I've also found it weird that I wasn't able to include files from the src folder in my unit tests since it was my understanding that this is the place where the actual program is to be implemented, while the lib directory only served as a place to put re-usable libraries (or things of that sort).
Apparently, you're meant to place everything that should be unit tested (which would be just about everything, really) inside the lib directory? Do I understand that correctly?

@jerabaul29
Copy link
Author

@Adrian-Samoticha this is what I ended up doing; though there is a (discouraged) optional platformio.ini flag to also include the src into the tests; see https://docs.platformio.org/en/latest/plus/unit-testing.html#unit-testing-shared-code and https://docs.platformio.org/en/latest/projectconf/section_env_test.html#id6 .

@ivankravets
Copy link
Member

Guys, thank you so much for your feedback and comments. Let us summarize your requests:

  1. Allow nested folder structure in the "test" folder
  2. Add support for build_type = test, the firmware will be built together with a testing framework
  3. Update test/README with information that source files from the src folder are not included in the test build by default.

Did I miss something?

@jcw
Copy link

jcw commented Apr 10, 2022

So there will be two ways to test?

  • pio test builds with lib/ and test/ but ignores src/
  • build_type = test builds with lib/ and src/ and ignores test/

I'd be fine either way, but this might lead to quite a bit of confusion.
Right now, I tend to test with that 1st approach, i.e. isolated units of the main code in lib/.
Is that 2nd test meant as a way to perform integration testing of a complete app?

@ivankravets
Copy link
Member

No, the workflow will be the same. build_type = test will just add testing framework to the build process (include paths, source files)

@ivankravets ivankravets added this to the 5.3.0 milestone Apr 11, 2022
@ivankravets ivankravets changed the title A few comments about unit testing :) Added support for test hierarchies Apr 22, 2022
@ivankravets
Copy link
Member

Guys, thanks for your feedback on PlatformIO Unit Testing. We refactored our unit testing engine from scratch. A lot of sweet things are coming soon. See https://github.com/platformio/platformio-core/milestone/94 and https://github.com/platformio/platformio-core/blob/develop/HISTORY.rst

Summary of what has been done:

  1. Added support for build_type = test
  2. Added support for test hierarchies (nested test suites). A test suite (folder) MUST start from test_ prefix. You can now write complex tests that include test-code related folders.

Please upgrade to the latest PIO Core via pio upgrade --dev and re-test these features. Does it work now?

Please also review https://github.com/platformio/platformio-core/milestone/94 , did we miss something?

@nomis
Copy link
Contributor

nomis commented May 19, 2022

All of the tests are now prefixed with test_ because the directories have to have that prefix, shouldn't that be removed from the display name automatically?

5.2.5

17:11:07.006  ========================== [PASSED] Took 0.60 seconds ==========================
17:11:07.006  
17:11:07.006  Test       Environment    Status    Duration
17:11:07.006  ---------  -------------  --------  ------------
17:11:07.006  millis     native         PASSED    00:00:00.847
17:11:07.006  printable  native         PASSED    00:00:00.604
17:11:07.006  ========================= 2 succeeded in 00:00:01.452 =========================

6.0.1

08:13:44.874  =================================== SUMMARY ===================================
08:13:44.874  Environment    Test            Status    Duration
08:13:44.874  -------------  --------------  --------  ------------
08:13:44.874  native         test_printable  PASSED    00:00:01.042
08:13:44.874  native         test_millis     PASSED    00:00:00.622
08:13:44.874  ================== 7 test cases: 7 succeeded in 00:00:01.664 ==================

@ivankravets
Copy link
Member

test_ignore and test_filter options depend on the full folder name of a test. Could it lead to the misunderstanding of what to use later for these options? For example, someone will try pio test -i printable and it will not work because the valid test path is test_printable.

Your thoughts?

@nomis
Copy link
Contributor

nomis commented May 19, 2022

The test_ignore and test_filter options could work on both names?

(Those options would need to continue to work with directory names because providing those on the command line with shell tab completion is useful.)

That will work until someone tries to do test_example and test_test_example but filter them differently.

@jcw
Copy link

jcw commented May 19, 2022

I'd vote for a single choice. Better get used to one long-term convention, IMO. Perhaps check for the mistake and give a clear notice?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants