Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41644 )
Change subject: tests/: Add spi.c unit tests ......................................................................
tests/: Add spi.c unit tests
Change-Id: Ie97c818c3f427bfb438ac3023fa4d1bfe1bec8f0 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M tests/meson.build A tests/spi.c M tests/tests.c M tests/tests.h 4 files changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/41644/1
diff --git a/tests/meson.build b/tests/meson.build index 0c21cb9..5af0a3e 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -2,6 +2,7 @@
srcs = [ 'tests.c', + 'spi.c', ]
mocks = [ diff --git a/tests/spi.c b/tests/spi.c new file mode 100644 index 0000000..fda3903 --- /dev/null +++ b/tests/spi.c @@ -0,0 +1,53 @@ +#include <include/test.h> +#include "tests.h" + +#include "programmer.h" + +#include <string.h> + + +static int mock_spi_send_command(const struct flashctx *flash, + unsigned int write_count, + unsigned int read_count, + const unsigned char *write_buffer, + unsigned char *read_buffer) +{ + return 0xAA55; +} + +void registered_masters_test_success(void **state) +{ + (void) state; /* unused */ + + static struct spi_master mst = { + .max_data_read = 16, + .max_data_write = 8, + .command = mock_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, + }; + assert_int_not_equal(register_spi_master(&mst), ERROR_FLASHROM_BUG); + + struct spi_master *master = ®istered_masters[0].spi; + assert_int_equal(master->max_data_read, 16); + assert_int_equal(master->max_data_write, 8); + assert_int_equal(master->command(NULL,0,0,NULL,NULL), 0xAA55); +} + +void programmer_init_test_success(void **state) +{ + (void) state; /* unused */ + + assert_int_equal( + programmer_init( + CONFIG_DEFAULT_PROGRAMMER, + strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS) + ), 0 + ); + + struct flashrom_flashctx flashctx = { 0, }; + int ret = probe_flash(®istered_masters[0], 0, &flashctx, 0); + assert_int_not_equal(ret, -1); +} diff --git a/tests/tests.c b/tests/tests.c index ded39de..338646e 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -18,5 +18,11 @@ { int ret = 0;
+ const struct CMUnitTest spi_tests[] = { + cmocka_unit_test(registered_masters_test_success), + cmocka_unit_test(programmer_init_test_success), + }; + ret |= cmocka_run_group_tests_name("spi.c tests", spi_tests, NULL, NULL); + return ret; } diff --git a/tests/tests.h b/tests/tests.h index b088e24..ccc3f5e 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -1,4 +1,7 @@ #ifndef TESTS_H #define TESTS_H
+void registered_masters_test_success(void **state); +void programmer_init_test_success(void **state); + #endif /* TESTS_H */
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/41644
to look at the new patch set (#2).
Change subject: tests/: Add spi.c unit tests ......................................................................
tests/: Add spi.c unit tests
BUG=b:157280555 BRANCH=none TEST=builds
Change-Id: Ie97c818c3f427bfb438ac3023fa4d1bfe1bec8f0 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M tests/meson.build A tests/spi.c M tests/tests.c M tests/tests.h 4 files changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/41644/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41644 )
Change subject: tests/: Add spi.c unit tests ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/flashrom/+/41644/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/41644/2//COMMIT_MSG@11 PS2, Line 11: builds but do the tests pass?
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c File tests/spi.c:
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@15 PS2, Line 15: 0xAA55 Magic number?
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@36 PS2, Line 36: NULL,0,0,NULL,NULL Shouldn't we have spaces around here?
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@44 PS2, Line 44: programmer_init( : CONFIG_DEFAULT_PROGRAMMER, : strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS) Why not use a variable for this one as well?
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@50 PS2, Line 50: , This comma shouldn't be needed
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@51 PS2, Line 51: ®istered_masters[0] registered_masters
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41644 )
Change subject: tests/: Add spi.c unit tests ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c File tests/spi.c:
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@15 PS2, Line 15: 0xAA55
Magic number?
Just something unique yes.
https://review.coreboot.org/c/flashrom/+/41644/2/tests/spi.c@44 PS2, Line 44: programmer_init( : CONFIG_DEFAULT_PROGRAMMER, : strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS)
Why not use a variable for this one as well?
I think these unit-tests are pretty incomplete and horrible so I would be prepared to rebase this all to the end of the series as there is a lot of state that needs to be mocked to properly test probe_flash()
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/41644
to look at the new patch set (#3).
Change subject: tests/: Add spi.c unit tests ......................................................................
tests/: Add spi.c unit tests
BUG=b:157280555 BRANCH=none TEST=builds
Change-Id: Ie97c818c3f427bfb438ac3023fa4d1bfe1bec8f0 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M tests/meson.build A tests/spi.c M tests/tests.c M tests/tests.h 4 files changed, 64 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/41644/3
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)