Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Peter Marheine.

View Change

10 comments:

• File tests/init_shutdown.c:

  • Patch Set #5, Line 217: if (strcmp(pathname, "/dev/i2c-254") != 0)

    Actually, given that this is test, let's make it an assert. In the case `assert_string_equal()` , full API is here https://api.cmocka.org/group__cmocka__asserts.html

    The reason is: if pathname is incorrect, assert will fail with a message like " <some string> != "/dev/i2c-254" line 217 init_shutdown.c" , and by looking at the line 217 it would be clear where the error comes from.

    Without assert, execution returns back to realtek_mst_i2c_spi.c and at some point init function fails, and the test fails with message like "0 != 1 line 29 init_shutdown.c". So you can only know that programmer_init failed and would need to spend more time debugging.

  • Patch Set #5, Line 219: if ((flags & O_RDWR) != O_RDWR)

    assert_int_equal

  • Patch Set #5, Line 229: if (fd != REALTEK_MST_MOCK_FD)

    assert_int_equal

  • Patch Set #5, Line 231: if (request != I2C_SLAVE)

    assert_int_equal

  • Patch Set #5, Line 235: if (addr != 0x4a)

    assert_int_equal

  • Patch Set #5, Line 243: if (fd != REALTEK_MST_MOCK_FD || sz != 1)

    assert_int_equal twice (for FD and for sz)

  • Patch Set #5, Line 250: if (fd != REALTEK_MST_MOCK_FD)

    assert_int_equal

  • Patch Set #5, Line 252: if (sz == 1 || sz == 2)

    assert_in_set

• File tests/io_mock.h:

  • Patch Set #5, Line 78: int (*open)(void *state, const char *pathname, int flags);

    In addition to my comment in tests.c: one more line here:
    	int (*open64)(void *state, const char *pathname, int flags);

• File tests/tests.c:

  • Patch Set #5, Line 93: return __wrap_open(pathname, flags);

    When I said "wrap doesn't need to redirect to another wrap" I meant this:

    	LOG_ME;
    	if (current_io && current_io->open64)
    		return current_io->open64(current_io->state, pathname, flags);
    	return MOCK_HANDLE;

    And then if the tests want to handle both open and open64 in the same way,
    	const struct io_mock realtek_mst_io = {
    		.open = realtek_mst_open,
                    .open64 = realtek_mst_open,
                      ......
            }

    The reason is: this layer (tests.c) is the same for all tests, I don't want to make assumptions what some test needs, test can decide. You are right, in most cases it doesn't matter, but let the test decide.

To view, visit change 56911. To unsubscribe, or for help writing mail filters, visit settings.