Feature/ci unit tests on push

[philj404]

Adds Makefiles and a GitHub action which will run arduino-timer unit tests in a Linux build environment.

A passing result helps give confidence that a code change/new feature did not cause a regression.

Information from a failing result can give hints on what to investigate.

See issue https://github.com/contrem/arduino-timer/issues/20 for more info.

-- This is a follow-on to https://github.com/contrem/arduino-timer/pull/19 . It adds continuous integration capability to regression testing.

Since it's a follow-on, you could just merge this PR and get the other one "for free". But since it's a different feature I was working on, I wanted to keep the two perspectives separate.

[contrem]

Please fix the warnings:

timerTest.ino: In member function ‘virtual void test_timer_at::once()’:
timerTest.ino:158:8: warning: unused variable ‘atTask’ [-Wunused-variable]
  158 |   auto atTask = timer.at(atTime, DummyTask::runATask, &waste_3ms);
      |        ^~~~~~
timerTest.ino: In member function ‘virtual void test_timer_in::once()’:
timerTest.ino:199:8: warning: unused variable ‘atTask’ [-Wunused-variable]
  199 |   auto atTask = timer.in(delayTime, DummyTask::runATask, &waste_3ms);
      |        ^~~~~~
timerTest.ino: In member function ‘virtual void test_timer_every::once()’:
timerTest.ino:241:8: warning: unused variable ‘everyTask1’ [-Wunused-variable]
  241 |   auto everyTask1 = timer.every(50, DummyTask::runATask, &waste_3ms);
      |        ^~~~~~~~~~
timerTest.ino:242:8: warning: unused variable ‘everyTask2’ [-Wunused-variable]
  242 |   auto everyTask2 = timer.every(200, DummyTask::runATask, &waste_100ms_once);
      |        ^~~~~~~~~~
timerTest.ino: In member function ‘virtual void test_timer_delayToNextEvent::once()’:
timerTest.ino:295:22: warning: comparison of integer expressions of different signedness: ‘long unsigned int’ and ‘int’ [-Wsign-compare]
  295 |   while (simMillis() < start + 1000) {
      |          ~~~~~~~~~~~~^~~~~~~~~~~~~~

This last one is important:

timerTest.ino: In member function ‘virtual void test_timer_delayToNextEvent::once()’:
timerTest.ino:295:22: warning: comparison of integer expressions of different signedness: ‘long unsigned int’ and ‘int’ [-Wsign-compare]

Ensure that anything getting or comparing values from simMillis matches its return type unsigned long. Same with the ticks and tick return types. Probably easiest to just use unsigned long for everything instead of int.

[philj404]

Done.

• I'm using unsigned long more consistently now.
• I also set the Makefile to stop the build on warnings.

Note these changes will now be less forgiving than just running your tests on an Arduino Uno. It's a different build environment with different default warnings.

Contributors writing unit tests should probably use a Linux/MacOS system and clone UnixHostDuino into Arduino/libraries.

While technically the GitHub Action will return error messages, it's an awkward feedback loop compared to just doing the (Linux) build/test cycle locally.

[contrem]

Don't forget your constructor arguments and class members:

    DummyTask(int runTime, bool repeats = true):
      busyTime(runTime), repeat(repeats)
    {
      reset();
    };

    int busyTime;
    int numRuns;
    int timeOfLastRun;

Could you explain your choice to use assertNear here, and not check for an absolute value? It's a fixed test, the value shouldn't change or fluctuate.

  assertNear(dt_3millisec.numRuns, 100, 9); // 7+ millisecs apart
  // (ideally 142 runs)

  assertNear(dt_5millisec.numRuns, 90, 4); // 11+ millisecs apart
  // (ideally 90 runs)

[philj404]

Thanks for reviewing my code at this level of detail. Yes those member variables should be unsigned long too -- although the test is over before it can notice.

There are a couple reasons I prefer to use assertNear() for counting the number of times a task ran.

1. I did not mathematically prove what the correct counts were for when these tasks interfere with each other.
2. Originally the test used delay() and millis() directly. I did not want the test to fail just because a processor was busy or extra-slow (on an arbitrary platform -- which might even be a virtual machine now).
3. I do not want to over-constrain the timer requirements. The timer's also a scheduler. I do want to ensure that no task starves. But it's OK for the test if the timer code changes to check the queue in reverse order, or do something with priority queues... which might change the number of runs slightly.
4. Right now I'm more interested in getting the unit test infrastructure working correctly.

TimerTest is more a smoke-test or proof-of-principle. It's a good start (and hopefully confirms correct behavior as far as it goes). It is not a thorough exercise of all arduino-timer features. But there should be enough of an example here to show what to do to add REAL tests you want.

[contrem]

Tracking in pj/ci-unit-tests.

Please close the other PR if this one contains it.

[contrem]

Merged in e12c01acc0d6c085178a1f7e50df54532f08feaa.

[philj404]

Thanks for doing this merge! It appears there may be an indentation conflict with default Arduino settings... which are not self-consistent anyway:

• The default auto-format indent size is 2. (Set in .../formatter.conf as indent=spaces=2 .)
• The indent size this library prefers is 4. (This is also the default Arduino tab setting. Set in .../preferences.txt as editor.tabs.size=2 .)

So... sorry if my contributions weren't quite conformant to your library's standards. I will be more careful next time.

That said, it looks like this pull request has been merged in with appropriate formatting. It is ready to close if you think you are done too.

[contrem]

Thanks for the info on setting the Arduino indentation settings, and thanks for helping improve the library.