Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons, Anastasia Klimchuk.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59237 )
Change subject: tests: Set up mock chip memory in consistent and predictable way
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/59237
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871
Gerrit-Change-Number: 59237
Gerrit-PatchSet: 7
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 25 Jan 2022 00:20:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59237 )
Change subject: tests: Set up mock chip memory in consistent and predictable way
......................................................................
Patch Set 7:
(2 comments)
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/59237/comment/e3b8045c_dad901b5
PS6, Line 33: .buf = { MOCK_CHIP_CONTENT },
> This only initialises the first element of `buf`, the rest of the array is initialised to 0. […]
Yes it is misleading: I misled myself :(
I will keep {0} here as it was before, that's a good idea. And yes, setup_chip is expected to be called at the beginning of every test anyway.
https://review.coreboot.org/c/flashrom/+/59237/comment/f0b9efa9_a3860d99
PS6, Line 97: memset(&g_chip_state.buf[0], MOCK_CHIP_CONTENT, MOCK_CHIP_SIZE);
> How about: […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/59237
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871
Gerrit-Change-Number: 59237
Gerrit-PatchSet: 7
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 25 Jan 2022 00:14:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Paul Menzel, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59237
to look at the new patch set (#7).
Change subject: tests: Set up mock chip memory in consistent and predictable way
......................................................................
tests: Set up mock chip memory in consistent and predictable way
This patch adds a macro MOCK_CHIP_CONTENT which represents a memory
state of a mock chip. The macro is used to initialise mock chip
memory at the beginning of a test (in setup_chip function).
Previously mock chip memory was not reset between tests. For
existing tests that did not matter, however new test for verify
operation (added later in this chain) needs mock chip memory to
be setup in a predictable way.
BUG=b:181803212
TEST=ninja test
Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M tests/chip.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/59237/7
--
To view, visit https://review.coreboot.org/c/flashrom/+/59237
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871
Gerrit-Change-Number: 59237
Gerrit-PatchSet: 7
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59238
to look at the new patch set (#7).
Change subject: tests: Add comprehensive comment for chip.c
......................................................................
tests: Add comprehensive comment for chip.c
The following describes the two mechanisms of testing done for
flash chip operations.
BUG=b:181803212
TEST=ninja test
Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M tests/chip.c
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/59238/7
--
To view, visit https://review.coreboot.org/c/flashrom/+/59238
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac
Gerrit-Change-Number: 59238
Gerrit-PatchSet: 7
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Paul Menzel, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59237 )
Change subject: tests: Set up mock chip memory in consistent and predictable way
......................................................................
Patch Set 6: Code-Review+1
(2 comments)
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/59237/comment/29291648_c2e54296
PS6, Line 33: .buf = { MOCK_CHIP_CONTENT },
This only initialises the first element of `buf`, the rest of the array is initialised to 0. I see it gets filled later on, but I find this potentially misleading.
I'd remove the static initialiser (or leave it to explicitly initialise to 0) and put the actual initialisation code in a helper function just below.
https://review.coreboot.org/c/flashrom/+/59237/comment/0541c335_3cc7c8fe
PS6, Line 97: memset(&g_chip_state.buf[0], MOCK_CHIP_CONTENT, MOCK_CHIP_SIZE);
How about:
memset(g_chip_state.buf, MOCK_CHIP_CONTENT, sizeof(g_chip_state.buf));
--
To view, visit https://review.coreboot.org/c/flashrom/+/59237
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871
Gerrit-Change-Number: 59237
Gerrit-PatchSet: 6
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 24 Jan 2022 13:40:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment