[
https://issues.apache.org/jira/browse/MESOS-7877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16165042#comment-16165042
]
Greg Mann commented on MESOS-7877:
----------------------------------
{code}
commit 1f4d7ef27e0e4936c1ea15d4e56d778e35a92507
Author: Gastón Kleiman gas...@mesosphere.io
Date: Wed Sep 13 09:21:20 2017 -0700
Added new overloads for the `createExecutorInfo` test helper method.
These new overloads make it possible to specify framework ID, executor
resources, and executor ID as a protobuf message rather than a string.
Review: https://reviews.apache.org/r/62197/
{code}
{code}
commit 2a6f6b7aedf05b23ae0fe04364159c87f6c5cea8
Author: Gastón Kleiman gas...@mesosphere.io
Date: Wed Sep 13 09:21:25 2017 -0700
Changed `EXPECT` to `ASSERT` when relying on the assertion afterwards.
A common pattern in our tests is to check that at least one offer is
received using:
'EXPECT_FALSE(offers->offers().empty())'
The test then accesses the first element of the array returned by
`offers->offers()` to extract information such as the agent ID.
This patch makes the tests that follow this pattern use `ASSERT_FALSE`
instead of `EXPECT_FALSE` to avoid invalid memory accesses when the
array is empty.
Review: https://reviews.apache.org/r/62042/
{code}
> Audit test code for undefined behavior in accessing container elements
> ----------------------------------------------------------------------
>
> Key: MESOS-7877
> URL: https://issues.apache.org/jira/browse/MESOS-7877
> Project: Mesos
> Issue Type: Bug
> Components: test
> Reporter: Benjamin Bannier
> Assignee: Gastón Kleiman
> Priority: Minor
> Labels: mesosphere, newbie, tech-debt, test
>
> We do not always make sure we never access elements from empty containers,
> e.g., we use patterns like the following
> {code}
> Future<vector<Offer>> offers;
> // Satisfy offers.
> EXPECT_FALSE(offers.empty());
> const auto& offer = (*offers)[0];
> {code}
> While the intention here is to diagnose an empty {{offers}}, the code still
> exhibits undefined behavior in the element access if {{offers}} was indeed
> empty (compilers might aggressively exploit undefined behavior to e.g.,
> remove "impossible" code). Instead one should prevent accessing any elements
> of an empty container, e.g.,
> {code}
> ASSERT_FALSE(offers.empty()); // Prevent execution of rest of test body.
> {code}
> We should audit and fix existing test code for such incorrect checks and
> variations involving e.g., {{EXPECT_NE}}.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
