Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
tests/: Add programmer enumerate correspondence test
The enumerate defines within programmer.h must be aligned with the programmer dispatch table within flashrom.c. The following unit-tests tries to improve coverage here to avoid issues like we saw when ENE and MEC1308 were upstreamed.
BUG=none BRANCH=none TEST=meson test
Change-Id: I13efc4561f743edf51326d4b408be8715b630377 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M tests/flashrom.c M tests/tests.c M tests/tests.h 3 files changed, 94 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/47356/1
diff --git a/tests/flashrom.c b/tests/flashrom.c index 50464dd..9180922 100644 --- a/tests/flashrom.c +++ b/tests/flashrom.c @@ -15,6 +15,47 @@
#include <include/test.h>
+/* must be before programmer.h include */ +#define CONFIG_INTERNAL 1 +#define CONFIG_DUMMY 1 +#define CONFIG_MEC1308 1 +#define CONFIG_NIC3COM 1 +#define CONFIG_NICREALTEK 1 +#define CONFIG_NICNATSEMI 1 +#define CONFIG_GFXNVIDIA 1 +#define CONFIG_RAIDEN 1 +#define CONFIG_DRKAISER 1 +#define CONFIG_SATASII 1 +#define CONFIG_ATAHPT 1 +#define CONFIG_ATAVIA 1 +#define CONFIG_ATAPROMISE 1 +#define CONFIG_IT8212 1 +#define CONFIG_FT2232_SPI 1 +#define CONFIG_SERPROG 1 +#define CONFIG_BUSPIRATE_SPI 1 +#define CONFIG_DEDIPROG 1 +#define CONFIG_DEVELOPERBOX_SPI 1 +#define CONFIG_ENE_LPC 1 +#define CONFIG_RAYER_SPI 1 +#define CONFIG_PONY_SPI 1 +#define CONFIG_NICINTEL 1 +#define CONFIG_NICINTEL_SPI 1 +#define CONFIG_NICINTEL_EEPROM 1 +#define CONFIG_OGP_SPI 1 +#define CONFIG_SATAMV 1 +#define CONFIG_LINUX_MTD 1 +#define CONFIG_LINUX_SPI 1 +#define CONFIG_LSPCON_I2C_SPI 1 +#define CONFIG_REALTEK_MST_I2C_SPI 1 +#define CONFIG_USBBLASTER_SPI 1 +#define CONFIG_MSTARDDC_SPI 1 +#define CONFIG_PICKIT2_SPI 1 +#define CONFIG_CH341A_SPI 1 +#define CONFIG_DIGILENT_SPI 1 +#define CONFIG_JLINK_SPI 1 +#define CONFIG_NI845X_SPI 1 +#define CONFIG_STLINKV3_SPI 1 + #include "programmer.h"
void flashbuses_to_text_test_success(void **state) @@ -49,3 +90,54 @@ bustype = BUS_NONE; assert_string_equal(flashbuses_to_text(bustype), "None"); } + +void validate_prog_enumerates_test_success(void **state) +{ + (void) state; /* unused */ + + enum programmer prog = PROGRAMMER_INTERNAL; + const char *prog_names[] = { + "internal", + "dummy", + "mec1308", + "nic3com", + "nicrealtek", + "nicnatsemi", + "gfxnvidia", + "raiden_debug", + "drkaiser", + "satasii", + "atahpt", + "atavia", + "atapromise", + "it8212", + "ft2232_spi", + "serprog", + "buspirate_spi", + "dediprog", + "developerbox", + "ene_lpc", + "rayer_spi", + "pony_spi", + "nicintel", + "nicintel_spi", + "nicintel_eeprom", + "ogp_spi", + "satamv", + "linux_mtd", + "linux_spi", + "lspcon_i2c_spi", + "realtek_mst_i2c_spi", + "usbblaster_spi", + "mstarddc_spi", + "pickit2_spi", + "ch341a_spi", + "digilent_spi", + "jlink_spi", + "ni845x_spi", + "stlinkv3_spi", + }; + for (; prog < PROGRAMMER_INVALID; prog++) { + assert_string_equal(programmer_table[prog].name, prog_names[prog]); + } +} diff --git a/tests/tests.c b/tests/tests.c index 3124607..d2a22a5 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -46,6 +46,7 @@
const struct CMUnitTest flashrom_tests[] = { cmocka_unit_test(flashbuses_to_text_test_success), + cmocka_unit_test(validate_prog_enumerates_test_success), }; ret |= cmocka_run_group_tests_name("flashrom.c tests", flashrom_tests, NULL, NULL);
diff --git a/tests/tests.h b/tests/tests.h index cb905fd..89fc7fa 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -27,6 +27,7 @@
/* flashrom.c */ void flashbuses_to_text_test_success(void **state); +void validate_prog_enumerates_test_success(void **state);
/* spi25.c */ void spi_write_enable_test_success(void **state);
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
Patch Set 2:
This change is ready for review.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
Patch Set 2: Code-Review+1
(2 comments)
https://review.coreboot.org/c/flashrom/+/47356/2/tests/flashrom.c File tests/flashrom.c:
https://review.coreboot.org/c/flashrom/+/47356/2/tests/flashrom.c@96 PS2, Line 96: (void) casts are unary operators, and shouldn't use a space
https://review.coreboot.org/c/flashrom/+/47356/2/tests/flashrom.c@99 PS2, Line 99: const char *prog_names const char *const prog_names
Hello Sam McNally, build bot (Jenkins), Victor Ding, Shiyu Sun, Angel Pons, Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/47356
to look at the new patch set (#3).
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
tests/: Add programmer enumerate correspondence test
The enumerate defines within programmer.h must be aligned with the programmer dispatch table within flashrom.c. The following unit-tests tries to improve coverage here to avoid issues like we saw when ENE and MEC1308 were upstreamed.
BUG=b:172668501 BRANCH=none TEST=`meson build && ninja -C build test` ran.
Change-Id: I13efc4561f743edf51326d4b408be8715b630377 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M tests/flashrom.c M tests/tests.c M tests/tests.h 3 files changed, 96 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/47356/3
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/flashrom/+/47356/2/tests/flashrom.c File tests/flashrom.c:
https://review.coreboot.org/c/flashrom/+/47356/2/tests/flashrom.c@99 PS2, Line 99: const char *prog_names
const char *const prog_names
Done
Shiyu Sun has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
Patch Set 3: Code-Review+1
Victor Ding has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
Patch Set 3: Code-Review+1
Attention is currently required from: Edward O'Callaghan. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47356 )
Change subject: tests/: Add programmer enumerate correspondence test ......................................................................
Patch Set 3:
(1 comment)
File tests/flashrom.c:
https://review.coreboot.org/c/flashrom/+/47356/comment/49c2b79a_631feaa9 PS3, Line 94: validate_prog_enumerates_test_success I don't quite understand, here is the test whether the order in program_table meets the definition of prog_names?
The order of programmer enum may still be wrong, especially when doing "make CONFIG_DEFAULT_PROGRAMMER=PROGRAMMER_XXX"
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Shiyu Sun (Code Review)
-
Victor Ding (Code Review)
-
Xiang Wang (Code Review)