dwyl / smart-home-firmware

Firmware for dwyl/smart-home-security-system
GNU General Public License v3.0
4 stars 0 forks source link

Testing firmware #1

Closed th0mas closed 4 years ago

th0mas commented 4 years ago

We need to learn how to properly test firmware and what's actually possible to test.

Nerves does have testing support, but not every library we will use will do.

We also need to build a working .travis.yml file that supports testing our firmware

th0mas commented 4 years ago

There doesn't seem to be a recognised way of unit testing the hardware side of nerves. Some people mock hardware using things like tty0tty but that requires a lot of knowledge of Unix and Linux that I don't have.

I'll write tests for all the pure Elixir parts of the application, although for I deliberately made this firmware as thin as possible (Its mostly glue code between a Phoenix channel and a NFC library) I'm not sure how much of this I'll be able to do

th0mas commented 4 years ago

Trying to launch tests launches the entire application with it.... we're using named processes so it crashes if we try and launch the same process twice :/

José Valim recommends being able to name your processes on an Elixir Forum Post

In my opinion, not starting your application in your tests because you want to manually start a process with the same name is somewhat a bad practice. Instead, I would make sure my API allows processes to be started with a custom name, so you do:

Worker.start_link(:my_name_during_test, ...) And I tend to use the test name as the name of the process itself:

test "my test", config do Worker.start_link(config.test, ...) end This works nicely if you are starting the process from setup too.

So looks like I've got some refactoring to do...

th0mas commented 4 years ago

Another good writeup by michalmuskala, who's on the Ecto core team:

One thing to additionally consider is testing. We’re spoiled in Elixir by the concurrency, so whenever I’m writing systems, I’m thinking how can I test them concurrently. This usually also turns out to be helpful when later scaling the system, but that’s another story.

Even, if I ultimately will run the process registered under a local name, I will rarely register the name directly in start_link - instead I will accept an argument with a name. This can be later used to pass the name from the supervisor to create a named actor for the production system and integration testing. At the same time, this allows easily spawning additional processes in tests for unit testing the particular process. This is a bit more verbose, because it requires passing in the process name/pid as an additional argument to the functions, but this isn’t that big price to be paid and can be worked around as well (the simplest solution being a default argument).

https://elixirforum.com/t/is-giving-a-genserver-module-as-name-a-good-practice/12261/4?u=t0mh

th0mas commented 4 years ago

All the firmware "internals" now have tests, but anything on the "boundry" -e.g. NFC reader & Socket callbacks are not.

Socket callbacks should be possible...

nelsonic commented 4 years ago

@th0mas if possible, please try and run the tests on Travis-CI as a reliable third party check. (if you get stuck please open issues with the specific things you face and I will try and help)

th0mas commented 4 years ago

@nelsonic Please try your CI magic on this - I can't get both tests and coverage to work

nelsonic commented 4 years ago

The first step will be getting it running on my localhost (or local hardware) so that I know what I'm debugging on Travis. 👍 I will take a look at it Monday morning. 📆

nelsonic commented 4 years ago

It appears that Travis is failing to compile scenic_driver_glfw: https://travis-ci.com/github/dwyl/smart-home-firmware/builds/177031095#L286 image

Going to try and add it to the Travis-CI VM: https://github.com/boydm/scenic_driver_glfw#installing-on-ubuntu

nelsonic commented 4 years ago

https://travis-ci.com/github/dwyl/smart-home-firmware/builds/177034398#L257

E: Command line option 'y' [from -y] is not understood in combination with the other options.

image

Removing it now. Actually, I didn't include the install keyword on the line. let's try that first!

nelsonic commented 4 years ago

https://travis-ci.com/github/dwyl/smart-home-firmware/builds/177034982#L208

E: Unable to locate package libglew2.0
E: Couldn't find any package by glob 'libglew2.0'
E: Couldn't find any package by regex 'libglew2.0'

image

Reading: https://packages.ubuntu.com/search?keywords=libglew2.0 libglew2-bionic

Going to update the distro to Bionic.

nelsonic commented 4 years ago

Apparently, sudo is required: (otherwise we get "Permission denied" error) https://travis-ci.com/github/dwyl/smart-home-firmware/jobs/364631847#L227

image

nelsonic commented 4 years ago

Ok, so the dependencies are all installed, but mix setup is not defined. https://travis-ci.com/github/dwyl/smart-home-firmware/builds/177036800#L717 image

You will see the same error if you attempt to run mix setup on your Mac:

Generated smart_home_firmware app
** (Mix) The task "setup" could not be found

Removing it from .travis.yml

nelsonic commented 4 years ago

Looks like we have progress: Build Status https://travis-ci.com/github/dwyl/smart-home-firmware/builds/177039695

image

:shipit:

nelsonic commented 4 years ago

@th0mas back to you. codecov.io (totally get that it's "WIP") 👍

th0mas commented 4 years ago

Trying to use GPIO on travis breaks the test.

This works on my mac, it seems to fallback to a stub implementation

image

th0mas commented 4 years ago

Going to spend some time working on integration tests with the heroku hub sever instance....

th0mas commented 4 years ago

Going to cheat on some of these tests and remove display/ from our coverage...

This only contains presentation logic, no business logic - which is tested elsewhere. Theres no testing framework/guide for scenic and mocking an entire graphics library for ~4 tests seems extreme

nelsonic commented 4 years ago

@th0mas that seems reasonable for now. 👍 But in the future we would prefer to have some kind of test for the display to check that it's not going haywire or breaking. 💭