Support custom `defines` for unittest definitions

[hlovdal]

In the platform definitions it is possible to inject custom preprocessor defines to be used, e.g.

  leonardo:
    board: ...
    package: ...
    gcc:
      features:
      defines:
        - MY_CUSTOM_DEFINE
      warnings:
      flags:

What I am missing is for this to be possible to do for unittest definitions (UNITTEST_SCHEMA) in .arduino-ci.yaml as well, e.g.

unittest:
  libraries:
    - SomeLibrary
  defines:
    - SKIP_LCD_TESTS    # all unit tests
  platforms:
    - uno
        defines:
          - USE_FAKE_KEYBOARD    # only uno unit tests
    - leonardo

In addition to defines a compiler_argument option would be applicable to be able to give arguments like -g -O0 --coverage --std=c++98 ....

Currently I am just adding ret += [ "-DUNIT_TEST", "-g", "--coverage" ] in test_args, but it would be great to not have to modify arduino_ci.

This would be applicable for compile as well.

[ianfixes]

The spirit of .arduino-ci.yml files is to convey settings that would remain static across many commits and/or PRs in the same project. E.g. skipping a set of tests that is known to be faulty and is waiting for dependency to be fixed, or whatnot.

Along those same lines, I'd prefer not to change the compiler arguments at all -- my feeling is that they should mimic the Arduino IDE's compile options to the extent possible (--std=c++98 or whatever it uses). Otherwise, I run the risk of the unit tests not reflecting the reality of the compiled sketches.

I'm confused at what you're after for the platform-specific stuff as well like USE_FAKE_KEYBOARD; unless I'm missing something, you would typically just do this logic inside the library by checking #ifdef on one of the constants that the individual platforms define (or one of their characteristics, like SERIAL3). Can you explain that use case a bit more?

Your points about SKIP_LCD_TESTS is well taken though. If I'm reading this correctly, you'd like a way to narrow down the tests that get run so that you can power through a small part of the codebase. I would probably add that as a command line option to the arduino_ci_remote.rb. Would that work? How do you see this as working differently than the --testfile-select=GLOB option that I added in #93?

[ianfixes]

I've pushed some updates to REFERENCE.md, I'm not sure if they address any of what you're asking for here.

[hlovdal]

Both USE_FAKE_KEYBOARD and SKIP_LCD_TESTS were made up examples while writing that comment so they do not have any deep meaning.

My current need is for differentiating between target compilation and unit test. I am using a FSM library that contains some static_assert safeguards that depends on #include <type_traits>. The library supports disabling the tests with a TINYFSM_NOSTDLIB define, and since type_traits is not available for target android I need to disable them. But type_traits is available when compiling unit tests and I definitely want them enabled in that case.

So putting TINYFSM_NOSTDLIB in defines for compile and not in unittest would be one scenario (although in my case I want tinyfsm' disable logic to be reversed, instead of enabled unless disabled I want it to be disabled unless enabled. This because I want to be able to compile in android studio or with vscode as well where the tests needs to be disabled out of the box. Therefore I have modified the header file to include

#ifndef UNIT_TEST
#define TINYFSM_NOSTDLIB
#endif

and enable UNIT_TEST only when running aduino_ci's unittest).

---

With regards to compiler options, --std=c++98 was another made up example from browsing gcc's manual page and is not a real need. But both -g and -O0 --coverage are something I always want to have as compiler options for unit tests. If my unit tests fail I want to be able to debug them with full source code, and I always want to keep track of what code is covered by unit test or not. Enabling this in .arduino-ci.yml might very well not be the right place though.

[ianfixes]

I do use -g for some things, but it looks like I could certainly make that unconditional:
https://github.com/ianfixes/arduino_ci/blob/master/lib/arduino_ci/cpp_library.rb#L299-L312

      arg_sets = []
      arg_sets << ["-std=c++0x", "-o", executable.to_s, "-DARDUINO=100"]
      if libasan?(gcc_binary)
        arg_sets << [ # Stuff to help with dynamic memory mishandling
          "-g", "-O1",
          "-fno-omit-frame-pointer",
          "-fno-optimize-sibling-calls",
          "-fsanitize=address"
        ]
      end
      arg_sets << test_args(aux_libraries, ci_gcc_config)
      arg_sets << cpp_files_libraries(aux_libraries).map(&:to_s)
      arg_sets << [test_file.to_s]
      args = arg_sets.flatten(1)

I'm unfamiliar with --coverage, but that sounds like an excellent thing to add -- can you create a new issue specifically for coverage? Mostly I'd like to focus the discussion there on getting an idea of what the output format would be and how you'd like to see it reported / artifacted.

As for the TINYFSM_NOSTDLIB, I'm a bit torn. Bluntly, it's out of scope; unless I'm missing something, the fact that (some) code packaged for Arduino can also be included in other C++ projects is mostly luck -- it was certainly not the goal of the Arduino developers to have interoperability between Arduino libs and regular libs (or else we'd be able to use the real boost library with Arduino and not a compromise). Nothing stops that interop problem from getting even worse over time.

While I'm very much in favor of the idea of creating a system where those dual-use libraries are possible (and easy), that's a very ambitious goal and this project isn't mature enough to just take that in stride. If and when this project gets to the point where we cover the Arduino hardware functions really well and achieve some sort of compiler-parity (i.e. when we run out of stuff like #102 and #72), then I will absolutely go after something like this.

But for now, I think that's too risky of a thing to chase after. Again, please reopen if I'm missing something important there.