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

Add Makefile & Docker for running tests locally #19981

Merged

Conversation

costas-basdekis
Copy link
Contributor

Description

  • Add Makefile with targets:
    • For a single or all tets
    • For CI, local, or docker
  • Add Dockerfile and docker-compose configuration

Running make gives:

Tasks for local development:
* tests-single-ci:             Run a single test from inside the CI
* tests-single-local:          Run a single test locally
* tests-single-local-docker:   Run a single test locally, using docker-compose
* tests-all-local:             Run all tests locally
* tests-all-local-docker:      Run all tests locally, using docker-compose
* setup-local-docker:          Setup local docker-compose

Benefits

There isn't a very clear way to run all tests locally, and this makes these commands easy to find and run.

Also, since different environments can have different Python versions, etc, a Dockerfile and a docker-compose configuration are provided, to standardise the running, regardless of where are they run

Configurations

No special configurations are necessary

Related Issues

Can't find a related issue

@costas-basdekis costas-basdekis force-pushed the local-docker-tests branch 4 times, most recently from 3ee124a to a00cb6f Compare November 1, 2020 13:24
@sjasonsmith
Copy link
Contributor

Very interesting! I will try this out today.

@sjasonsmith
Copy link
Contributor

I have been playing with this PR this morning. You can find a whole discussion on Discord here: https://discordapp.com/channels/461605380783472640/496524752211410952/772558074912571442

This seems to be working really well. It is much easier to use than trying to figure out how to script together tests for local runs on your own. Until now I have still been using a Travis file along with something called What Would Travis Do?!

I'm using this on Windows, though so there is one huge caveat. Docker runs in WSL2. WSL2 has terrible performance when accessing files from the "Windows" drive. Each test took about 3 minutes, versus 45 seconds when using WSL1 (with my old method)

By moving my Marlin folder into the "Linux" drive it is really fast, about 25 seconds per test.

I haven't tried this yet, but here is an article from Microsoft describing how to configure VS Code to use the Windows front-end and WSL2 back-end. That should be able to provide what feels native on Windows, but gives you proper WSL2 performance.
https://code.visualstudio.com/blogs/2020/03/02/docker-in-wsl2

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
buildroot/tests/run_tests Outdated Show resolved Hide resolved
buildroot/tests/run_tests Show resolved Hide resolved
@rhapsodyv
Copy link
Member

@costas-basdekis first of all: it's a great PR and it will be a very good addition to Marlin.

I would like to ask a few questions:

1 - Why docker? I mean, PIO already setup for us the development environment in our actual platform. This allow us to just run the tests locally, without any VM or Docker.

2 - Why Makefile? The standard user (I think even the most advanced ones) just use PIO to build and test. Maybe we should just update the test scripts to be able to easily run it. Or, even more fun: integrate with VS Code or PIO, to run the tests.

3 - PIO have some support for testing. I never took a look myself. It have support for unit testing, for example. Maybe we should take a look if there anything we can integrate. https://docs.platformio.org/en/latest/plus/unit-testing.html

* Add Makefile with targets:
  * For a single or all tets
  * For CI, local, or docker
* Add Dockerfile and docker-compose configuration
@costas-basdekis
Copy link
Contributor Author

Thanks @rhapsodyv!

  1. The problem for me was that, even though I can compile Marlin for my printer, I somehow couldn't do the same for tests in WSL. Also, I've had bad experiences in the past with different libraries/packages being broken on the OS, that Docker assures me that everything runs exactly as it's supposed to run - regardless of local config/OS/packages versions.
  2. That's another habit I've picked up: a Makefile gives you a list of available common commands, and you don't have to figure out how to get set up locally, run tests, etc. It's one more thing that made development easy for me for Distinct runout pin-states #19965, because I had to first figure out how to run tests, and then remember the steps when I closed my terminal
  3. I'll have a look at that and see what we can do. As I mentioned in other discussions, I'm not a C++ developer, but would be happy to help any way I can and skill up. I would appreciate any ideas as well.

In the end, Docker & Makefile have made my (and other teammates') lives very easy, because they standardise and make it easy for newcomers to find their bearings - and I went through a frustrating couple of hours today trying to figure out why platformio wasn't playing nice, before giving up.

Also since Docker is optional, if your local setup is working fine, this should make things easier anyway.

Let me know what you think

@sjasonsmith
Copy link
Contributor

I think I like the Docker approach. I've certainly had issues because I was happily using Python 2 until one script in one environment started needing Python 3 features.

@thinkyhead thinkyhead added C: Build / Toolchain T: Development Makefiles, PlatformIO, Python scripts, etc. labels Nov 2, 2020
@thinkyhead
Copy link
Member

thinkyhead commented Nov 3, 2020

I've just been using my mftest script (included in buildroot/share/git) to run tests as needed. This seems to be more comprehensive, though.

@costas-basdekis
Copy link
Contributor Author

costas-basdekis commented Nov 4, 2020

I didn't realise that those scripts were there @thinkyhead

I'm happy to merge those scripts, or pick only one version. The reason I added the ones I did, is that I wanted to match closely the ones that run on CI, and they seem to be quite simpler than the mftest ones. But happy to discuss

@thinkyhead
Copy link
Member

thinkyhead commented Nov 4, 2020

The main advantage of mftest is that it can run the test in place, but it would be cool if it could present the same list of available tests and give the option to run the selected test in place or under the Docker. If I want to just invoke one test (by index) from one test file (by name) and have it run under this new system, what would be the command-line (*nix) to make that happen?

Currently you can run a single test in place using a command like:

mftest mega2560 1 -y

@costas-basdekis
Copy link
Contributor Author

@thinkyhead

I haven't added this here

For example, if you wanted to run BigTreeTech GTR 8 Extruders with Auto-Fan and Mixed TMC Drivers from BIGTRE_GTR_V1_0 you would run:

make tests-single-local TEST_TARGET=BIGTRE_GTR_V1_0 ONLY_TEST="BigTreeTech GTR 8 Extruders with Auto-Fan and Mixed TMC Drivers"

A bit shorter:

make tests-single-local TEST_TARGET=BIGTREE_GTR_V1_0 ONLY_TEST="mixed TMC"

Which means if you want to test mixed TMC drivers for all tests (which matches 5 tests total) you can do:

make tests-all-local ONLY_TEST="mixed TMC"

Of course that's still more verbose than mftest BIGTREE_GTR_V1_0 1 -y, but perhaps we could add/repurpose shortcuts.

@thinkyhead
Copy link
Member

If we can make the script accept an integer value for "ONLY_TEST" then mftest can simply pass on user input to the make command to run the selected test. Meanwhile, I'll see what it looks like to use the test description text as identifier.

@costas-basdekis
Copy link
Contributor Author

@thinkyhead

I've now added the ability to use the index of the test in a file:

make tests-single-local TEST_TARGET=BIGTREE_GTR_V1_0 ONLY_TEST=1

If ONLY_TEST is a 1-digit or 2-digit number, we'll assume it's an index and only target that one

@thinkyhead thinkyhead force-pushed the bugfix-2.0.x branch 3 times, most recently from 4274255 to a97a1ae Compare November 14, 2020 02:06
* Add Makefile with targets:
  * For a single or all tets
  * For CI, local, or docker
* Add Dockerfile and docker-compose configuration
To do that use `ONLY_TEST=<The description of the test>`
@costas-basdekis
Copy link
Contributor Author

@thinkyhead We could be using PlatformIO custom targets and get rid most of the mf* scripts, in favour of something like platformio run -t marlin-conf-test etc.

I've done something like that for unit tests in one of my own branches

@thinkyhead thinkyhead merged commit 1cceae8 into MarlinFirmware:bugfix-2.0.x Nov 18, 2020
@costas-basdekis
Copy link
Contributor Author

Thanks for the merge @thinkyhead !

Let me know if I can support any further in the various scripts we currently employ, perhaps in using PIO to simplify and convert shell scripts & makefile scripts to PIO Python commands

FhlostonParadise pushed a commit to FhlostonParadise/Marlin that referenced this pull request Nov 21, 2020
@thinkyhead
Copy link
Member

@costas-basdekis — Any possibility we can move some of the items that were added to the root folder to buildroot instead (without breaking anything)?

@costas-basdekis
Copy link
Contributor Author

Thanks for the merge!

@thinkyhead We could move everything except the Makefile, since otherwise you would have to do make -C buildroot/... which makes it awkward.

Also, if we move the docker-compose.yml you would also have to do docker-compose -f buildroot/... which again is awkward.

So my suggestion is to move everything except Makefile and docker-compose.yml.

Kannix2005 pushed a commit to Kannix2005/Marlin-1 that referenced this pull request Dec 7, 2020
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request Dec 9, 2020
tharts pushed a commit to tharts/Marlin that referenced this pull request Jan 6, 2021
dpreed pushed a commit to dpreed/Marlin_2.0.x that referenced this pull request Feb 5, 2021
dpreed pushed a commit to dpreed/Marlin_2.0.x that referenced this pull request Feb 5, 2021
kpishere pushed a commit to kpishere/Marlin that referenced this pull request Feb 19, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Apr 28, 2021
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Build / Toolchain T: Development Makefiles, PlatformIO, Python scripts, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants