Attention is currently required from: Felix Singer, Thomas Heijligen, Alexander Goncharov.
10 comments:
Patchset:
Nice work, thanks for the patch!
File tests/ch341a_spi.c:
transfer->status = LIBUSB_TRANSFER_COMPLETED;
transfer->actual_length = transfer->length;
transfer->callback(transfer);
Are you sure these lines needed? What happens if you remove them, and just return 0?
Maybe I am missing something, but I looked through the programmer code and I can't find where status, actual_length and callback are used?
Especially, calling callback in a mock should not be needed, since return value is not used (or maybe it's void).
Patch Set #5, Line 43: mediatek_i2c_spi_basic_lifecycle_test_success
This is a different test probably a copy-paste ;)
You need to use your test method name here, `ch341a_spi_basic_lifecycle_test_success`
The meaning behind this is: if the programmer is disabled in the build, the corresponding test should not run either (since there is nothing to test).
File tests/libusb_wraps.c:
The new added wraps should go into a separate patch pointing to this one.
Why? In all previous tests, wraps were added in the same patch with the test that needed those wraps.
Patch Set #5, Line 39: /* https://libusb.sourceforge.io/api-1.0/group__libusb__lib.html#ga5f8376b7a863a5a8d5b8824feb8a427a8 */
I'm going to delete such links after everyone says they're OK with the patch. […]
Alright, thank you! I will just mark this unresolved so that you won't forget to remove links at the last moment :)
You need 2 tabs here as indentation (see below __wrap_libusb_get_config_descriptor in the same situation)
Patch Set #5, Line 196: calloc(1, sizeof(struct libusb_transfer));
This is a custom logic (a small one but still), so it should go into custom mocks. Same as you did for __wrap_libusb_submit_transfer.
Patch Set #5, Line 219: free(transfer);
Same here, custom logic, please move to io_mock, and implement in tests/ch341a_spi.c
File tests/usb_unittests.h:
Patch Set #5, Line 28: CONFIG_CH341A_SPI == 1
You can run tests with this option enabled and disabled. When you run with option disabled, the test will be SKIPPED, but everything should be built and tests run (so, no errors).
To try that:
In your builddir you can set option disabled, run
meson configure -Dprogrammer=<list some programmers, but not ch341a>
for example
meson configure -Dprogrammer=dediprog,linux_mtd,linux_spi
Then run tests, and check that tests for the rest of programmers are skipped as expected
Then to return back with everything enabled you can run
meson configure -Dprogrammer=auto or
meson configure -Dprogrammer=all
depending on what was you previous setting
This will also verify that SKIP_TEST macro is set up correctly.
Patch Set #5, Line 58: struct libusb_transfer;
If you add typedef like the others, you can avoid repeating `struct` in wraps.
To view, visit change 67664. To unsubscribe, or for help writing mail filters, visit settings.