Arduino-CI / arduino_ci

Unit testing and Continuous Integration (CI) for Arduino libraries, from a Ruby gem
Apache License 2.0
110 stars 34 forks source link

Advice on using (mocking?) dependencies of the Library Under Test #150

Closed jgfoster closed 4 years ago

jgfoster commented 4 years ago

I'd like to test my use of the LiquidCrystal library

I believe that I can use other libraries and that they should "just work" because they would make calls to the base Arduino code that you currently implement. For example, displaying something on the LCD should set appropriate pins and I could check the pins using the existing Godmode interface. But with that approach, I'd have to reverse engineer the meaning of the pins and watch for appropriate values (in fact, I've tried this at https://github.com/jgfoster/Arduino/tree/master/LCD1602).

Alternatively, it would be nice to have something like the Serial interface available through Godmode. For that I suppose I could take the existing library code and modify it to keep track of high-level events and make them available for analysis. Is that the right approach or am I missing something obvious?

If I did create such a mock library, would it be something that should be contributed back to this project?

ianfixes commented 4 years ago

Have you looked at https://github.com/ianfixes/arduino_ci/blob/master/REFERENCE.md#serial-data ?

If you're talking about something that could provide a more semantic explanation of raw bits being sent to a device, the closest thing I have to what you're talking about is the ability to read pins as streams of bytes instead of streams of bits. That's crudely defined in the form of a toAscii method: https://github.com/ianfixes/arduino_ci/blob/master/REFERENCE.md#pin-history-as-ascii

This is already messy, because the grouping of bits into bytes depends on the offset of the first byte's first bit, and endian-ness in the stream.

More long term, I think the path forward is going to be a more robust approach to Godmode. Does #145 cover some of what you're looking for?

jgfoster commented 4 years ago

The Serial example isn't quite what I want since Serial is part of the built-in Arduino system. At first I thought that a better example would be SoftwareSerial since it is in the list of external libraries. I see that there are tests for this "library", but it appears that it isn't really one of the Arduino Libraries, but is in some sense built-in. I also see that, instead of using the "real" library, you have a substitute SoftwareSerial.h that deals directly with GodmodeState instead of letting the official code read and write to pins.

I'm sure this is more useful, but it doesn't give me an example of how to interact with a true Arduino Library that has C++ code in the location returned by bundle exec arduino_library_location.rb. I see that there is a discussion of custom versions of external arduino libraries, but I can't find any examples.

I have a custom library that uses LiquidCrystal. It works from the Arduino IDE with a trivial sketch, but doesn't work from arduino_ci:

Unit testing test.cpp with g++... 

Last command:  $ g++ -std=c++0x -o /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/unittest_test.cpp.bin -DARDUINO=100 -g -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address -D__AVR_ATmega328P__ -I/Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/vendor/bundle/ruby/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino -I/Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/vendor/bundle/ruby/2.6.0/gems/arduino_ci-0.3.0/cpp/unittest -I/Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/src /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/vendor/bundle/ruby/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/Arduino.cpp /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/vendor/bundle/ruby/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/Godmode.cpp /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/vendor/bundle/ruby/2.6.0/gems/arduino_ci-0.3.0/cpp/arduino/stdlib.cpp /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/vendor/bundle/ruby/2.6.0/gems/arduino_ci-0.3.0/cpp/unittest/ArduinoUnitTests.cpp /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/src/HelloLCD.cpp /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/test/test.cpp

/Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/src/HelloLCD.cpp:4:10: fatal error: 'LiquidCrystal.h' file not found
#include <LiquidCrystal.h>
         ^~~~~~~~~~~~~~~~~
1 error generated.
/Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/test/test.cpp:3:10: fatal error: 'LiquidCrystal.h' file not found
#include <LiquidCrystal.h>
         ^~~~~~~~~~~~~~~~~
1 error generated.
...Unit testing test.cpp with g++                                              ✗

The library is present in the expected directory:

jfoster@pitcairn HelloLCD % ls -alF `bundle exec arduino_library_location.rb`
total 24
drwxr-xr-x  11 jfoster  staff   352 Sep  9 10:58 ./
drwxr-xr-x@ 12 jfoster  staff   384 Sep  9 09:20 ../
-rw-r--r--@  1 jfoster  staff  6148 Sep  9 08:55 .DS_Store
drwxr-xr-x  14 jfoster  staff   448 Sep  8 18:00 Blink/
lrwxr-xr-x   1 jfoster  staff    57 Sep  9 10:52 DoSomething@ -> /Users/jfoster/code/arduino_ci/SampleProjects/DoSomething
drwxr-xr-x  13 jfoster  staff   416 Sep  9 10:57 HelloLCD/
drwxr-xr-x   7 jfoster  staff   224 Sep  8 21:46 LiquidCrystal/
drwxr-xr-x   8 jfoster  staff   256 Sep  5 20:13 RingBuffer/
lrwxr-xr-x   1 jfoster  staff    59 Sep  9 10:58 TestSomething@ -> /Users/jfoster/code/arduino_ci/SampleProjects/TestSomething
drwxr-xr-x   7 jfoster  staff   224 Sep  8 11:36 USBHost/
-rw-r--r--   1 jfoster  staff    87 Sep  2 18:51 readme.txt
jfoster@pitcairn HelloLCD % 

It seems to me that the g++ command (above) does not provide an include directory or a reference to the desired C++ file.

It looks like we are supposed to be able to use an external library, but I can't get it to work. Any advice?

ianfixes commented 4 years ago

The first thing that I notice is /Users/jfoster/code/arduino_ci/SampleProjects/HelloLCD/src/HelloLCD.cpp, which sets off a red flag because I expect HelloLCD to be in the directory specified by bundle exec arduino_library_location.rb.

Beyond that, it sounds like you're asking how to test a library that depends on another 3rd-party Arduino library. I suspect my documentation on this is misleading. I had set things up so that you (according to REFERENCE.md) specify the library dependencies in the configuration file.

Snip of that text:

unittest:

  # These dependent libraries will be installed
  libraries:
    - "abc123"
    - "def456"

These files get included in C++ compilation (code here and then here).

So in your case the dependent library would be LiquidCrystal.

My comment should be updated

- # These dependent libraries will be installed
+ # These dependent libraries will be included from Arduino's "Libraries" directory during compilation.
+ #    If they don't exist there already, the Arduino IDE will be used to attempt to install them
    libraries:
      - "abc123"
      - "def456"

But there's probably a better place in the docs to explain all of the behavior related to dependencies on other 3rd-party libraries.

Is this answering the entire question or just part of it?

jgfoster commented 4 years ago

I thought I had tried all that and it didn't work, but now, reading and following the instructions carefully (!), it seems to be finding the files. I'll keep working at it. Thanks for your help!

jgfoster commented 4 years ago

Part of the reason for developing inside arduino_ci is that I anticipate making changes to the testing framework. In particular, I'd like to examine your suggestion that "the path forward is going to be a more robust approach to Godmode." As to #145, it seems sufficiently vague that I'm not sure how it would handle my situation. I think I'll pursue "the simplest thing that could possibly work" and see where it leads me.

ianfixes commented 4 years ago

It sounds like we can close this issue.

As far as work on Godmode is concerned, there are 2 facets being discussed right now. The first is that (currently) arduino_ci has no real capability of mimicking real-world hardware: bits take time to flip, functions take time to run, a circuit might be expected to produce certain effects on input pins, and so on.

The other issue is as you noted -- validation of a library that makes calls to a hardware driver can expect to run into a LOT of bits that aren't necessarily easy to verify in the current framework. Anything involving hardware that acts in asynchronous fashion would be cumbersome to say the least.

Having better use cases would lead to arduino_ci developing more ergonomic test patterns, but it's a chicken-and-egg problem for now (not enough adoption of this project to see where its real weaknesses are).

jgfoster commented 4 years ago

My only question is where would like questions related to testing LiquidCrystal? Is it okay to keep a conversation going on a closed issue (it's fine with me)? For example:

I find that I'd like a way to distinguish between when code is compiled from the Arduino IDE and when code is compiled from arduino_ci. Ideal would be something like #define ARDUINO_CI but the closest I've come is MOCK_PINS_COUNT. Thoughts?

ianfixes commented 4 years ago

Sounds like it's premature to close this issue 😄 I'm also opening #159 to track the discussion of constants.

So if I'm understand the question here, it's that you have one library that depends on another library, and you want to mock one out?

One way I can imagine doing this is to put mock classes directly in the library you wish to mock, so that anyone using the library can have access to them. That would mean writing the library such that it could handle dependency injection of some kind.

Failing that, you could create the mock library in its own Arduino library directory and declare mock versions of all the things you'd need access to. In that setup, your configuration would look something like this:

compile:
  libraries:
    - "LiquidCrystal"

unittest:
  libraries:
    - "LiquidCrystal Mock"

That at least gives you the ability to mock a library without any access to the original.

jgfoster commented 4 years ago

I've come up with hybrid approach that seems to work (step 1 of "make it work, make it right, make it fast"). I'm subclassing the standard library to create a testable object, and using a check in my primary program to decide whether to use the production or the test version (this is where #159 will be helpful). So, in a sense this isn't a pure mock (but could be turned into one by not being a subclass). If you think this could be done in another way that is more consistent with the general approach (and possibly be included here), let me know.

The code is LiquidCrystal_CI and a video describing it is available. Advice and comments are welcome, but I'll close this since we at least have a way to use/mock standard libraries.

ianfixes commented 4 years ago

From the video I can definitely see how important #159 is (to the readability of that #ifdef MOCK_PINS_COUNT).

But I think you've stumbled on something bigger here, which is that at some point this project will have to figure out what the best practices are for developing a library that you expect others to unit test with. So I'd prefer to leave this open as a way to track some possible solutions to these use cases:

1. Ship your own mock with your own library

As an author of a library that others bring in as a dependency (e.g. LiquidCrystal), you could provide your own mock class -- using the same public interface for the "regular" class.

2. Make a mock for a library whose author didn't provide one

As an author of a library that consumes a dependency (e.g. LiquidCrystal, assuming you aren't its author), you could create your own separate mock library subdirectory (e.g. LiquidCrystal_CI).

3. Use a mock library in your tests

As an author of a library that consumes a dependency, you have to manage the setup of that library in both local and CI environments.

jgfoster commented 4 years ago

This is a very nice summary and clarifies some of the murky areas I was struggling with. I have a few random thoughts:

matthijskooijman commented 4 years ago

Superclass for production, subclass for testing: I got this working but still feels a bit messy (#ifdef to choose between the superclass and the subclass).

This is an often-used approach for this, especially in bigger projects that use OO-concepts like virtual methods, factories, dependency injections, which can make this approach more feasible. Not so ideal for Arduino code, though.

Also, unless the functions are virtual functions, it gets more complicated to override them (I had to override the calls). Why aren't all functions virtual (like in my beloved Smalltalk!)? Is it because there is a size and/or performance cost?

Yup, virtual calls add memory for the vtables, runtime overhead for the method dispatch and prevent inlining of methods. All unless the compiler manages to devirtualize calls, which is also happening more often nowadays, though.

I can imagine you can work around this by using a VIRTUAL_IF_TESTING macro the expands to virtual when testing or nothing when in production, and just smack this macro onto all methods.

One other approach I used recently is to apply some trickery in a Makefile-based build so I could just replace entire files for the test build, so I could replace a production class with a testing class. That's quite powerful, but also not so granular and making small API changes to the production code breaks the test code quickly. And it's harder to mock a class in one testcase, but not in another.

I also recently came across this chapter from the book "Working effectively with legacy code", which explains the concept of "seams" (points where you can change behavior of code and/or replace dependencies), which might be insightful for this discussion.

Finally, I found Powerfake, a framework that uses the gcc linker's --wrap option to replace specific functions at link time, where the replacement function normally just calls the original function, but can be redirected to do something else at runtime. I haven't tried it yet, but that seems like a terribly effective way to do very specific and partial overrides of code (that could maybe replace, but more probably coexist with my suggestion of configurable behavior objects in #145). For completeness, I also came across Isolator++ which does a similar thing but completely at runtime, using hotpatching of function entry points, which sounds like it's even more awesome, but it's not open source (though it is free of charge, so maybe you do get to look at the source maybe).

ianfixes commented 4 years ago

There are a lot of good points being made here and I think it will be easiest to answer them by taking a step back and talking about context.

This project began several years ago because I was trying to write some code for a friend['s robot] that ran on Arduino with a Futaba remote control. I was going to have to make contributions to both the robot code and the Futaba library, and I realized that there was slim to no chance that the Futaba maintainers would accept contributions from a total stranger because in order to do so they would have to commit to manually testing all my changes, or blindly accept my edits. So I went looking for Free Software projects that would enable me to demonstrate the changes via unit testing... and found none.

So in 2020 we are just approaching the point where I wanted to be back then: a coherent system for unit testing libraries, including those that depend on other libraries. Some of what's taken so long is a need to make sure that what I wrote is (a) understandable to and (b) sufficient for the community I'm aiming it at. If people can't feel 100% confident in the testing framework, they're not going to trust the tests and no amount of development or test writing will improve my chances at the original goal of allowing Arduino library maintainers to accept contributions from complete strangers through the power of automated validation.

So, with that in mind

How important is it to make it easy to support testing with dependencies?

Existential importance; I considered this feature to be the MVP of the entire effort (otherwise I couldn't test the robot library with mocked Futaba). I would encourage anyone/everyone to reject arduino_ci if it couldn't handle this kind of use case.

Am I the first to have made a serious effort?

I can't say. You're the first one to both make a serious effort and share it back to me with comments about the experience 😄 . I have some work done in this area, but hesitated to call it a final draft -- I don't have enough data on how well library maintainers believe it suits their needs.

If the Arduino community doesn't quickly embrace automated testing, then maybe we just create an "Arduino CI" group/organization on GitHub and gradually port the popular libraries.

I'm full of patience in this area. The Arduino community is made up of both hardware and software experts, with varying degrees of crossover in those skills -- as software professionals, we tend to overlook how confusing the C compiler is when writing application code for the first time (let alone sorting out unit tests). I would love to support a group to do this though; my initial push was to get the Adafruit folks on board with this form of test-driven development (since they are very publicly open-source in their efforts).

Maybe we just build everything into Godmode. I suppose that the value of keeping things small and modular is less important when running in the development environment where we have fast CPUs and lots of RAM.

An awesome end goal for this project would be to influence the main Arduino project to support dependency injection at its core. In other words, have setup() and loop() accept arguments that provide the complete information of the hardware capabilities, and access to them (void setup(hardwareInfo) { } and void loop(hardwareSupport) { } for instance; digitalWrite() becomes hardwareSupport.digitalWrite()). Right now Godmode is just the end-run around the need to do that, similar to how the LiquidCrystal_CI mock would ideally trick the code under test into thinking that it was the "real" library.

By the way, did you notice how LiquidCrystal_CI does not (explicitly) refer to Godmode? Is this good (keeping things modular and self-contained, avoiding bloat in a single god-like class) or bad (introducing a new approach, adding to the cognitive load) or both?

Your approach is precisely the one I had in mind when I wrote this system, I just (so far) had trouble finding evidence that anyone else would find that approach to be a good one. I really appreciate the validation there. In my ideal world, library maintainers would be providing their own mocks so you don't have to write your own... but wherever they end up existing, that is the best way I can envision it working. Like you said, it would be untenable to track hundreds of pin events caused by a higher-level library.

Testing non-production code: Whenever the tested code is different from production, we have the risk that production will behave differently. This seems inherent in the mock or wrapper approach, so calls for some caution.

Totally agree, the ability to keep parity is the biggest liability of this project as a whole.

using the Wiki would eventually be good for a design discussion

In the meantime I'm informing the design from actual use cases (SampleProjects) so that I can ensure that the framework supports them -- I run them through the CI process. So for now I envision design discussions to play out as pull request discussions with focus on physical examples.

Powerfake, Isolator++

Any stuff I pull in here has to work exclusively under-the-hood. Arduino is aimed at hobbyists and beginners, and I want to ensure that we don't break the experience by putting in so many compiler tweaks that it embrittles the framework. Remember, the core of this project is still a homespun set of C++ libraries that are intended to act as a believable fake for Arduino libraries that already exist in the wild. Adding complexity to how compilation is done makes it more likely that we will run into our ultimate liability of being a different compilation system than the IDE.

jgfoster commented 4 years ago

I appreciate the background and the validation that my struggles stem not just from my ignorance but from this being somewhat uncharted territory.

I'm informing the design from actual use cases (SampleProjects) so that I can ensure that the framework supports them -- I run them through the CI process.

Would there be value in adding LiquidCrystal_CI to SampleProjects? If so, how would it be done? The conceptual leap is that LiquidCrystal itself isn't tested, nor is LiquidCrystal_CI, but just HelloLCD.

But as I'm writing this I have an idea. Maybe I should merge LiquidCrystal and LiquidCrystal_CI into one library containing both classes and write tests for both the high-level testing API (that would exist only if ARDUINO_CI_UNITTEST_ENVIRONMENT were defined and also write tests for the low-level pins. This might be tedious but still practical—how many pins does it take to set the display on or off, maybe 20? (See ReadPins.ino for some Arduino code that does just that.) Hmm... Sounds like a good project for an undergraduate class!

Then, LiquidCrystal_CI becomes a swap-in replacement for LiquidCrystal that works both in production (with identical code/size/speed to the prior library) and in test, has built-in tests, and HelloLCD becomes little more than a demo project (much like Blink).

Then LiquidCrystal_CI could become just another directory in SampleProjects, though that would raise the question of how to distribute the new library. Perhaps the CI for arduino_ci should copy some "approved" external libraries into SampleProjects as part of the testing. Though that risks making your project's tests depend on my project's validity.

ianfixes commented 4 years ago

Would there be value in adding LiquidCrystal_CI to SampleProjects?

I would put a minimal example in SampleProjects, but I'm wondering now if this project should really be a GitHub project that contains several repositories (some of which are standalone demonstration projects). All in all, as long as I can accomplish the goal of showing each use case in practice then that should serve both the documentation and testability goals.

jgfoster commented 4 years ago

I wouldn't want to detract from your "brand", but yes, I think that this calls for several projects that are closely affiliated rather than one that has everything. On the other hand, perhaps it would be enough (at least for starters) to have a central directory so that "my" project is just a click away from "yours". Perhaps you should check to see if arduino_ci is taken as a group/organization.

jgfoster commented 4 years ago

For a hint of some of my ideas about observing events, see this branch.

ianfixes commented 4 years ago

After some thought, I'm leaning toward creating a GItHub organization specifically for this. That will provide the most obvious path for growth and allow a set of repositories to be logically grouped. At the very least, I'd like that group to include working examples of each of the different situations we've discussed in this thread, with some room for expansion into the space of "contributing changes [including mocks] to existing libraries to make them more testable".

I'm not sure that LiquidCrystal would be an obvious choice for inclusion in this GitHub organization, but certainly a stripped-down version that helps illustrate the "bare minimum" architectural decisions would be a valuable tutorial for someone to copy from.

If possible, I'd like to get @per1234 's comments on this idea before going ahead.

jgfoster commented 4 years ago

If you are thinking about line-drawing (what is in, what is out), it might be reasonable to have another project for libraries. It does seem to me that until automated testing is more widely adopted, it will be helpful to have a place to put forks that have adopted testing. In any case, I anticipate having a fork of LiquidCrystal that works with Arduino CI, and I can host it on my own account.

per1234 commented 4 years ago

I think it's a great idea to have dedicated repositories for the libraries, whether that ends up being forks of the libraries that contain the mocks alongside the production code or mock-only companion libraries.

It makes sense to have an organization to collect all the repositories related to arduino_ci, rather than mixing them in among your unrelated repositories.

The one advantage of keeping example projects in this repository is it allows the sample projects to be updated in the same commit as the changes to arduino_ci that require the change. It's a bit more challenging to keep things synced up across repositories. I have no idea how often that sort of sync would be needed.

On the other hand, dedicated repositories for the examples will eliminate any concerns that a lot of large examples clutter up the arduino_ci repo. I also think they could make the examples a bit more accessible to users and contributors.


@jgfoster said:

it appears that it isn't really one of the Arduino Libraries, but is in some sense built-in.

SoftwareSerial is a platform bundled library. Except for the boards platforms (AKA "cores" or "packages") that extend another platforms, each platform bundles its own version of SoftwareSerial. The version bundled with the platform of the board being compiled for is used. The reason is that the code is architecture-specific, so it will be different from one platform to another. The API is compatible so the user isn't even aware of this.

Actually SoftwareSerial isn't a great example because developers of platforms for boards that have sufficient hardware serial capabilities may not bother to write a SoftwareSerial library. Better examples are the platform bundled libraries SPI or Wire, because these are always essential, and always very different under the hood. If universal versions of these libraries were published under the arduino-libraries organization, the code would be a bunch of completely separate implementations wrapped in preprocessor conditionals for each architecture.


@ianfixes said:

Is it straightforward to contribute such mocks back to library maintainers on github? What code patterns in those libraries would make this contribution more difficult or less difficult?

I think it is likely to be challenging. For the last few years, I have been making some efforts at promoting the use of CI for compile testing alone. Even though setting that up is often not much more than a copy/paste of a GitHub Actions workflow or .travis.yml with a couple of lines adjusted, this practice is not common in the Arduino community and too often when you do see it set up, the CI build has either broken or never worked in the first place.

I think some library authors will be receptive to this idea, but also that some PRs will remain open indefinitely or even be rejected due to the author not wanting to take on the maintenance and support burden for mocks they won't even learn to use. So it's nice to have a place to collect those library forks so the mocks are still accessible to arduino_ci users even if the upstream library won't provide them.

jgfoster commented 4 years ago

it might be reasonable to have another project for libraries.

I meant to say that it might be reasonable to have another organization/group for libraries (like arduino-libraries).

@per1234 said:

[automated testing] is not common in the Arduino community

While I'm a bit surprised that this is the case, I'm not disappointed since it gives me a chance to make a contribution to the community. I'm working with a biology professor who has cut the cost of his ocean acidification lab equipment by 99% ($15,000 to $150) by building his own aquarium controllers, and it seems to me that to have this be taken seriously it needs automated testing.

ianfixes commented 4 years ago

Thank you all for these ideas. One thing that's certainly been missing from this project over the last year is some concept of direction or path for growth, and I'm hearing very viable ones being proposed.

It's true that we will have to keep some sample projects in the library itself to ensure that there is some synchronization. It's also true that for end users of the arduino_ci Gem, a standalone repository is a necessity (the sample projects directory is just plain confusing, as jgfoster found out the hard way). Fortunately, as far as keeping standalone sample projects in sync, you have a specific version of the gem in your Gemfile and we would just keep an eye on whether each repository was specifying the latest released version.

I don't have any specific vision beyond the "core" ruby gem repo and a collection of example projects. I can imagine a few ways in which we might host a repository of mocks or forks of other libraries. But I think that can't happen without establishing some credibility on what does a well-formed and testable Arduino library look like?

So I'll consider the next phase of arduino_ci development to be an attempt to answer that question. #161 will track the transition to an organization.

jgfoster commented 4 years ago

I'm happy with the direction we are going and believe this question has been answered adequately. Since I asked should I close it or is there some value in keeping it open?

ianfixes commented 4 years ago

I'll open a new issue specifically on the topic of establishing what a "well formed" repository for an Arduino library should look like