Attention is currently required from: Sam McNally, Daniel Campello.
10 comments:
Patchset:
Thanks!
File tests/io_mock.h:
Patch Set #7, Line 63: /* Maximum number of open to mock. */
The number is probably arbitrary? If yes, you can add this clarification into a comment, "the number is arbitrary".
This way, if in future we need more calls, it will be easier to increase the number (no need to search history for the reasons why the number is this).
If you are in a good mood, let's make this number power of two :) like 4 for example.
Patch Set #7, Line 118: open_state
Another thing: could you please add a comment above this line, to explain what that is, something like
"An alternative to custom open mock, a test can register one of: either its own mock open function or fallback_open".
Patch Set #7, Line 118: struct io_mock_open_state *open_state;
This is, as I understand from the code, an alternative to custom open mocking, and mutually exclusive (a test can either mock open function or setup open state). Let's have more descriptive naming for this.
For example:
struct fallback_open_state *fallback_open
I remember we agreed with Edward on "fallback" name in some earlier patch, let's use this name.
File tests/io_mock.c:
Patch Set #7, Line 24: assert_true(io == NULL || io->open == NULL || io->open_state == NULL);
I would also add a comment above this line, to make it crystal clear:
"A test can register only one of: either custom mock for open function, or fallback open state".
File tests/tests.c:
Patch Set #7, Line 77: wrap_open_and_friends
Lets just call it "mock_open"
Could you please add new line between two conditionals? It would be easier to read.
Also here, please add new line between declaration of flags and asserts
Also here, please add new line before return statement.
To view, visit change 63227. To unsubscribe, or for help writing mail filters, visit settings.