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 unit testing framework #26948

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
ec14521
Add exec_tests_failing, to test unhappy paths
costas-basdekis Nov 1, 2020
0e946ff
Add ability to run unit tests
costas-basdekis Nov 3, 2020
b7c3462
Update tooling to work with modern Platform I/O and Marlin
sjasonsmith Apr 8, 2024
da97377
Refactor test mechanism to make tests more self-discovering and easie…
sjasonsmith Apr 8, 2024
9648cd8
Update READMEs
sjasonsmith Apr 8, 2024
e855bb8
Add to test-builds workflow
sjasonsmith Apr 9, 2024
ea5076d
Rename job
sjasonsmith Apr 9, 2024
41f0dcc
Restrict to bugfix-2.1.x branch, update comments.
sjasonsmith Apr 9, 2024
53ce692
whitespace, formatting, etc.
thinkyhead Apr 9, 2024
f3ad255
Split CI testing into two workflows
sjasonsmith Apr 9, 2024
903d5da
Use INI files inside test folders instead of CPP files
sjasonsmith Apr 9, 2024
8889e4f
Use config.ini
sjasonsmith Apr 9, 2024
e5ed773
Remove unused code
sjasonsmith Apr 9, 2024
efc9b54
parser.parse() does not accept const
sjasonsmith Apr 9, 2024
9484cb3
Revert unrelated changes
sjasonsmith Apr 9, 2024
68fac74
Update README to reflect config.ini change
sjasonsmith Apr 9, 2024
5514777
TEMPORARY changes to test github workflows
sjasonsmith Apr 9, 2024
28ec011
Revert unrelated changes
sjasonsmith Apr 9, 2024
791ddf3
Avoid main conflict with LINUX_HAL
sjasonsmith Apr 9, 2024
e0eee1a
Avoid SIGSEGV on timer cleanup
sjasonsmith Apr 9, 2024
3b6e647
Remove parse_line test
sjasonsmith Apr 9, 2024
c014c62
Revert "TEMPORARY changes to test github workflows"
sjasonsmith Apr 9, 2024
9e9a008
move YML
thinkyhead Apr 9, 2024
2767706
readme updates
thinkyhead Apr 9, 2024
6d90aab
rename
thinkyhead Apr 9, 2024
187e186
table format
thinkyhead Apr 9, 2024
6e7b10a
not force-pushed
thinkyhead Apr 9, 2024
55d836e
sort CI tests
thinkyhead Apr 9, 2024
e3da66e
test name from file
thinkyhead Apr 9, 2024
c0cbc71
not force-pushed
thinkyhead Apr 9, 2024
10c81cc
move string tests
thinkyhead Apr 9, 2024
296048d
tests-code => unit-test
thinkyhead Apr 9, 2024
fd351a9
split up string test
thinkyhead Apr 9, 2024
4118f36
fix docker "make" invocation
thinkyhead Apr 9, 2024
9d6f47d
string test
thinkyhead Apr 9, 2024
6a4be39
tweak readme
thinkyhead Apr 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/workflows/test-builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,47 @@ jobs:
- name: Run ${{ matrix.test-platform }} Tests
run: |
make tests-single-ci TEST_TARGET=${{ matrix.test-platform }}

# This runs all unit tests as a single job. While it should be possible to break this up into
# multiple jobs, they currently run quickly and finish long before the compilation tests.
run_unit_tests:
name: Run All Unit Tests
# These tests will only be able to run on the bugfix-2.1.x branch, until the next release
# pulls them into additional branches.
if: github.repository == 'MarlinFirmware/Marlin' && github.ref == 'refs/heads/bugfix-2.1.x'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I tested that the workflow worked on my own repo/branch, I didn't test this github.ref check. I'm attempting to prevent this from running on the 2.1.x branch, since it won't work yet.

If we don't want to gamble whether this will work once merged, we could split this into a new YML file with different branch requirements. I thought it made sense to add it to this file because then it can re-use all the trigger configuration regarding file paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That github.ref syntax came from GitHub Copilot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also move this to its own action, if that helps to make things cleaner.


runs-on: ubuntu-latest

steps:
- name: Check out the PR
uses: actions/checkout@v4

- name: Cache pip
uses: actions/cache@v4
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
restore-keys: |
${{ runner.os }}-pip-

- name: Cache PlatformIO
uses: actions/cache@v4
with:
path: ~/.platformio
key: ${{ runner.os }}-${{ hashFiles('**/lockfiles') }}

- name: Select Python 3.9
uses: actions/setup-python@v5
with:
python-version: '3.9'
architecture: 'x64'

- name: Install PlatformIO
run: |
pip install -U platformio
pio upgrade --dev
pio pkg update --global

- name: Run All Unit Tests
run: |
make tests-code-all-local
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ applet/
.clang_complete
.gcc-flags.json
/lib/
test/tmp*c

# Secure Credentials
Configuration_Secure.h
Expand Down
30 changes: 28 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ help:
@echo "make tests-single-local-docker : Run a single test locally, using docker"
@echo "make tests-all-local : Run all tests locally"
@echo "make tests-all-local-docker : Run all tests locally, using docker"
@echo "make setup-local-docker : Build the local docker image"
# @echo "make tests-code-single-ci : Run a single code test from inside the CI"
# @echo "make tests-code-single-local : Run a single code test locally"
# @echo "make tests-code-single-local-docker : Run a single code test locally, using docker-compose"
@echo "make tests-code-all-local : Run all code tests locally"
@echo "make tests-code-all-local-docker : Run all code tests locally, using docker-compose"
@echo "make setup-local-docker : Setup local docker-compose"
@echo ""
@echo "Options for testing:"
@echo " TEST_TARGET Set when running tests-single-*, to select the"
Expand Down Expand Up @@ -54,8 +59,29 @@ tests-all-local-docker:
@if ! $(CONTAINER_RT_BIN) images -q $(CONTAINER_IMAGE) > /dev/null ; then $(MAKE) setup-local-docker ; fi
$(CONTAINER_RT_BIN) run $(CONTAINER_RT_OPTS) $(CONTAINER_IMAGE) $(MAKE) tests-all-local VERBOSE_PLATFORMIO=$(VERBOSE_PLATFORMIO) GIT_RESET_HARD=$(GIT_RESET_HARD)

#tests-code-single-ci:
# export GIT_RESET_HARD=true
# $(MAKE) tests-code-single-local TEST_TARGET=$(TEST_TARGET)

# TODO: How can we limit tests with ONLY_TEST with platformio?
#tests-code-single-local:
# @if ! test -n "$(TEST_TARGET)" ; then echo "***ERROR*** Set TEST_TARGET=<your-module> or use make tests-code-all-local" ; return 1; fi
# platformio run -t marlin_$(TEST_TARGET)

#tests-code-single-local-docker:
# @if ! test -n "$(TEST_TARGET)" ; then echo "***ERROR*** Set TEST_TARGET=<your-module> or use make tests-code-all-local-docker" ; return 1; fi
# @if ! $(CONTAINER_RT_BIN) images -q $(CONTAINER_IMAGE) > /dev/null ; then $(MAKE) setup-local-docker ; fi
# $(CONTAINER_RT_BIN) run $(CONTAINER_RT_OPTS) $(CONTAINER_IMAGE) $(MAKE) tests-code-single-local TEST_TARGET=$(TEST_TARGET) ONLY_TEST="$(ONLY_TEST)"

tests-code-all-local:
platformio run -t test-marlin -e linux_native_test

tests-code-all-local-docker:
@if ! $(CONTAINER_RT_BIN) images -q $(CONTAINER_IMAGE) > /dev/null ; then $(MAKE) setup-local-docker ; fi
$(CONTAINER_RT_BIN) run $(CONTAINER_RT_OPTS) $(CONTAINER_IMAGE) $(MAKE) tests-code-all-local

setup-local-docker:
$(CONTAINER_RT_BIN) build -t $(CONTAINER_IMAGE) -f docker/Dockerfile .
$(CONTAINER_RT_BIN) buildx build -t $(CONTAINER_IMAGE) -f docker/Dockerfile .

PINS := $(shell find Marlin/src/pins -mindepth 2 -name '*.h')

Expand Down
5 changes: 4 additions & 1 deletion Marlin/src/HAL/LINUX/hardware/Timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ Timer::Timer() {
}

Timer::~Timer() {
timer_delete(timerid);
if (timerid != 0) {
timer_delete(timerid);
timerid = 0;
}
}

void Timer::init(uint32_t sig_id, uint32_t sim_freq, callback_fn* fn) {
Expand Down
2 changes: 2 additions & 0 deletions Marlin/src/HAL/LINUX/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
*/

#ifdef __PLAT_LINUX__
#ifndef UNIT_TEST

//#define GPIO_LOGGING // Full GPIO and Positional Logging

Expand Down Expand Up @@ -135,4 +136,5 @@ int main() {
read_serial.join();
}

#endif // UNIT_TEST
#endif // __PLAT_LINUX__
58 changes: 0 additions & 58 deletions Marlin/src/gcode/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,64 +331,6 @@ FORCE_INLINE bool is_M29(const char * const cmd) { // matches "M29" & "M29 ", b
return m29 && !NUMERIC(m29[3]);
}

#define PS_NORMAL 0
#define PS_EOL 1
#define PS_QUOTED 2
#define PS_PAREN 3
#define PS_ESC 4

inline void process_stream_char(const char c, uint8_t &sis, char (&buff)[MAX_CMD_SIZE], int &ind) {

if (sis == PS_EOL) return; // EOL comment or overflow

#if ENABLED(PAREN_COMMENTS)
else if (sis == PS_PAREN) { // Inline comment
if (c == ')') sis = PS_NORMAL;
return;
}
#endif

else if (sis >= PS_ESC) // End escaped char
sis -= PS_ESC;

else if (c == '\\') { // Start escaped char
sis += PS_ESC;
if (sis == PS_ESC) return; // Keep if quoting
}

#if ENABLED(GCODE_QUOTED_STRINGS)

else if (sis == PS_QUOTED) {
if (c == '"') sis = PS_NORMAL; // End quoted string
}
else if (c == '"') // Start quoted string
sis = PS_QUOTED;

#endif

else if (c == ';') { // Start end-of-line comment
sis = PS_EOL;
return;
}

#if ENABLED(PAREN_COMMENTS)
else if (c == '(') { // Start inline comment
sis = PS_PAREN;
return;
}
#endif

// Backspace erases previous characters
if (c == 0x08) {
if (ind) buff[--ind] = '\0';
}
else {
buff[ind++] = c;
if (ind >= MAX_CMD_SIZE - 1)
sis = PS_EOL; // Skip the rest on overflow
}
}

/**
* Handle a line being completed. For an empty line
* keep sensor readings going and watchdog alive.
Expand Down
60 changes: 60 additions & 0 deletions Marlin/src/gcode/queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,66 @@

#include "../inc/MarlinConfig.h"


#define PS_NORMAL 0
#define PS_EOL 1
#define PS_QUOTED 2
#define PS_PAREN 3
#define PS_ESC 4

inline void process_stream_char(const char c, uint8_t &sis, char (&buff)[MAX_CMD_SIZE], int &ind) {

if (sis == PS_EOL) return; // EOL comment or overflow

#if ENABLED(PAREN_COMMENTS)
else if (sis == PS_PAREN) { // Inline comment
if (c == ')') sis = PS_NORMAL;
return;
}
#endif

else if (sis >= PS_ESC) // End escaped char
sis -= PS_ESC;

else if (c == '\\') { // Start escaped char
sis += PS_ESC;
if (sis == PS_ESC) return; // Keep if quoting
}

#if ENABLED(GCODE_QUOTED_STRINGS)

else if (sis == PS_QUOTED) {
if (c == '"') sis = PS_NORMAL; // End quoted string
}
else if (c == '"') // Start quoted string
sis = PS_QUOTED;

#endif

else if (c == ';') { // Start end-of-line comment
sis = PS_EOL;
return;
}

#if ENABLED(PAREN_COMMENTS)
else if (c == '(') { // Start inline comment
sis = PS_PAREN;
return;
}
#endif

// Backspace erases previous characters
if (c == 0x08) {
if (ind) buff[--ind] = '\0';
}
else {
buff[ind++] = c;
if (ind >= MAX_CMD_SIZE - 1)
sis = PS_EOL; // Skip the rest on overflow
}
}


sjasonsmith marked this conversation as resolved.
Show resolved Hide resolved
class GCodeQueue {
public:
/**
Expand Down
7 changes: 7 additions & 0 deletions Marlin/tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
These test files are executed through the unit-tests built from the <root>/test folder.

By placing these outside of the main PlatformIO test folder, we are able to collect
all test files and compile them into multiple PlatformIO test binaries. This enables
tests to be executed against a variety of Marlin configurations.

To execute these tests, refer to the top-level Makefile.
37 changes: 37 additions & 0 deletions Marlin/tests/gcode/test_gcode.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#include "../test/marlin_tests.h"
#include <src/gcode/gcode.h>
#include <src/gcode/parser.h>

MARLIN_TEST(gcode, process_parsed_command) {
GcodeSuite suite;
parser.command_letter = 'G';
parser.codenum = 0;
suite.process_parsed_command(false);
}

MARLIN_TEST(gcode, parse_g1_xz)
{
char current_command[] = "G0 X10 Z30";
parser.command_letter = -128;
parser.codenum = -1;
parser.parse(current_command);
TEST_ASSERT_EQUAL('G', parser.command_letter);
TEST_ASSERT_EQUAL(0, parser.codenum);
TEST_ASSERT_TRUE(parser.seen('X'));
TEST_ASSERT_FALSE(parser.seen('Y'));
TEST_ASSERT_TRUE(parser.seen('Z'));
TEST_ASSERT_FALSE(parser.seen('E'));
}

MARLIN_TEST(gcode, parse_g1_nxz) {
char current_command[] = "N123 G0 X10 Z30";
parser.command_letter = -128;
parser.codenum = -1;
parser.parse(current_command);
TEST_ASSERT_EQUAL('G', parser.command_letter);
TEST_ASSERT_EQUAL(0, parser.codenum);
TEST_ASSERT_TRUE(parser.seen('X'));
TEST_ASSERT_FALSE(parser.seen('Y'));
TEST_ASSERT_TRUE(parser.seen('Z'));
TEST_ASSERT_FALSE(parser.seen('E'));
}
44 changes: 44 additions & 0 deletions Marlin/tests/queue/test_parse_line.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#include "../test/marlin_tests.h"
#include <unity.h>
#include <src/gcode/queue.h>

static void test_parse_line(const char *command, const char *expected) {
char buffer[MAX_CMD_SIZE];
int buffer_index = 0;
uint8_t input_state = PS_NORMAL;

for (int position = 0 ; command[position] ; position++) {
process_stream_char(command[position], input_state, buffer, buffer_index);
}
buffer[buffer_index] = 0;

TEST_ASSERT_EQUAL_STRING(expected, buffer);
}

MARLIN_TEST(parse_line, g1_x_and_parenthesis_comment)
{
#if ENABLED(PAREN_COMMENTS)
test_parse_line("G0 X10 (Z30)", "G0 X10 ");
#else
test_parse_line("G0 X10 (Z30)", "G0 X10 (Z30)");
#endif
}

MARLIN_TEST(parse_line, g1_x_and_parenthesis_inline_comment)
{
#if ENABLED(PAREN_COMMENTS)
test_parse_line("G0 X10 (Y20) Z30", "G0 X10 Z30");
#else
test_parse_line("G0 X10 (Y20) Z30", "G0 X10 (Y20) Z30");
#endif
}

MARLIN_TEST(parse_line, g1_xz)
{
test_parse_line("G0 X10 Z30", "G0 X10 Z30");
}

MARLIN_TEST(parse_line, g1_x_and_comment)
{
test_parse_line("G0 X10 ; Z30", "G0 X10 ");
}
14 changes: 14 additions & 0 deletions Marlin/tests/runout/test_runout_sensor.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#include "../test/marlin_tests.h"

#if ENABLED(FILAMENT_RUNOUT_SENSOR)

#include <src/feature/runout.h>

MARLIN_TEST(runout, poll_runout_states)
{
FilamentSensorBase sensor;
// Expected default value is one bit set for each extruder
uint8_t expected = static_cast<uint8_t>(~(~0u << NUM_RUNOUT_SENSORS));
TEST_ASSERT_EQUAL(expected, sensor.poll_runout_states());
}
#endif
Loading