Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[utest]: Allow the linker to remove any part of utest if not used #2559

Merged
merged 14 commits into from
Sep 10, 2016
Merged

[utest]: Allow the linker to remove any part of utest if not used #2559

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
acc3115
Move utest global serial object into a function.
pan- Aug 26, 2016
2bc5c3d
Move global Timeout object from utest_shim in static function.
pan- Aug 26, 2016
8fa239c
Move constant definitions of non POD object into cpp file.
pan- Aug 26, 2016
6d319c5
Construct control_t from a POD struct.
pan- Aug 26, 2016
7a6edda
Enhance compatibility between control_t and base_control_t:
pan- Aug 26, 2016
4bf28d4
Convert case_control to a POD.
pan- Aug 26, 2016
eed52a0
Replace default_handlers value by a reference.
pan- Aug 26, 2016
daa135a
Move defaults and handlers variable into a static function.
pan- Aug 26, 2016
df3e3b2
Fix scope of the serial variable.
pan- Aug 26, 2016
85bce14
Improve compatibility between base_control_t and control_t.
pan- Aug 26, 2016
a9d9987
Fix a rare case of imcompatibility between base_control_t and control_t.
pan- Aug 26, 2016
cb4a7fa
Use greentea_serial SingletonPtr.
pan- Aug 31, 2016
caa4c4f
Use a SingletonPtr for the Timeout object in utest_shim.
pan- Sep 6, 2016
aab1070
Use SingletonPtr for the internal variables defaults and handlers in …
pan- Sep 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion TESTS/storage_abstraction/basicAPI/basicAPI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ control_t test_initialize(const size_t call_count)
TEST_ASSERT(rc >= ARM_DRIVER_OK);
if (rc == ARM_DRIVER_OK) {
TEST_ASSERT_EQUAL(1, capabilities.asynchronous_ops);
return (call_count < REPEAT_INSTANCES) ? (CaseTimeout(200) + CaseRepeatAll) : CaseNext;
return (call_count < REPEAT_INSTANCES) ? (CaseTimeout(200) + CaseRepeatAll) : (control_t) CaseNext;
}

TEST_ASSERT(rc == 1);
Expand Down
1 change: 1 addition & 0 deletions features/frameworks/utest/source/utest_default_handlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const handlers_t utest::v1::verbose_continue_handlers = {
verbose_case_failure_handler
};

const handlers_t& utest::v1::default_handlers = greentea_abort_handlers;

// --- SPECIAL HANDLERS ---
static void test_failure_handler(const failure_t failure) {
Expand Down
61 changes: 34 additions & 27 deletions features/frameworks/utest/source/utest_harness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace

const Case *case_current = NULL;
size_t case_index = 0;
control_t case_control = control_t(REPEAT_SETUP_TEARDOWN);
base_control_t case_control = { REPEAT_SETUP_TEARDOWN, TIMEOUT_UNDECLR };
size_t case_repeat_count = 1;

void *case_timeout_handle = NULL;
Expand All @@ -47,8 +47,15 @@ namespace
size_t case_failed = 0;
size_t case_failed_before = 0;

handlers_t defaults = default_handlers;
handlers_t handlers = defaults;
handlers_t& defaults() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can handlers_t not be a class to and use the SingletonPtr paradigm for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handlers_t is not a class, it is a POD.
The cause of the trouble is related to the initialization of these handlers: the initialization is made by copy of default_handlers.
This is not something done at compile time, the initialization happen at runtime.
That mean that the variables defaults and handlers will be kept even if there is no code using them after the initialization.
That also means that the variable default_handlers will be kept and the various function pointers which initialize it will be kept.

It is not possible to use directly a SingletonPtr here because there is no way to specify the initialization sequence.
On the other hand, it can be worked around by creating a new class derived from handlers_t which default initialize to default_handlers . Do you prefer this solution ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, it can be worked around by creating a new class derived from handlers_t which default initialize to default_handlers . Do you prefer this solution ?

That would be great!

static handlers_t _handlers = default_handlers;
return _handlers;
}

handlers_t& handlers() {
static handlers_t _handlers = default_handlers;
return _handlers;
}

location_t location = LOCATION_UNKNOWN;

Expand Down Expand Up @@ -110,10 +117,10 @@ bool Harness::run(const Specification& specification)
return false;
test_cases = specification.cases;
test_length = specification.length;
defaults = specification.defaults;
handlers.test_setup = defaults.get_handler(specification.setup_handler);
handlers.test_teardown = defaults.get_handler(specification.teardown_handler);
handlers.test_failure = defaults.get_handler(specification.failure_handler);
defaults() = specification.defaults;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigh, C++, the only language this line could make sense in...

handlers().test_setup = defaults().get_handler(specification.setup_handler);
handlers().test_teardown = defaults().get_handler(specification.teardown_handler);
handlers().test_failure = defaults().get_handler(specification.failure_handler);

test_index_of_case = 0;
test_passed = 0;
Expand All @@ -127,16 +134,16 @@ bool Harness::run(const Specification& specification)
int setup_status = 0;
failure_t failure(REASON_NONE, location);

if (handlers.test_setup) {
setup_status = handlers.test_setup(test_length);
if (handlers().test_setup) {
setup_status = handlers().test_setup(test_length);
if (setup_status == STATUS_CONTINUE) setup_status = 0;
else if (setup_status < STATUS_CONTINUE) failure.reason = REASON_TEST_SETUP;
else if (setup_status > signed(test_length)) failure.reason = REASON_CASE_INDEX;
}

if (failure.reason != REASON_NONE) {
if (handlers.test_failure) handlers.test_failure(failure);
if (handlers.test_teardown) handlers.test_teardown(0, 0, failure);
if (handlers().test_failure) handlers().test_failure(failure);
if (handlers().test_teardown) handlers().test_teardown(0, 0, failure);
test_cases = NULL;
exit(1);
return true;
Expand All @@ -150,8 +157,8 @@ bool Harness::run(const Specification& specification)
scheduler.post(run_next_case, 0);
if (scheduler.run() != 0) {
failure.reason = REASON_SCHEDULER;
if (handlers.test_failure) handlers.test_failure(failure);
if (handlers.test_teardown) handlers.test_teardown(0, 0, failure);
if (handlers().test_failure) handlers().test_failure(failure);
if (handlers().test_teardown) handlers().test_teardown(0, 0, failure);
test_cases = NULL;
exit(1);
return true;
Expand All @@ -167,8 +174,8 @@ void Harness::raise_failure(const failure_reason_t reason)
if (test_cases == NULL) return;

utest::v1::status_t fail_status = STATUS_ABORT;
if (handlers.test_failure) handlers.test_failure(failure_t(reason, location));
if (handlers.case_failure) fail_status = handlers.case_failure(case_current, failure_t(reason, location));
if (handlers().test_failure) handlers().test_failure(failure_t(reason, location));
if (handlers().case_failure) fail_status = handlers().case_failure(case_current, failure_t(reason, location));

{
UTEST_ENTER_CRITICAL_SECTION;
Expand All @@ -184,25 +191,25 @@ void Harness::raise_failure(const failure_reason_t reason)
}

if (fail_status == STATUS_ABORT || reason & REASON_CASE_SETUP) {
if (handlers.case_teardown && location != LOCATION_CASE_TEARDOWN) {
if (handlers().case_teardown && location != LOCATION_CASE_TEARDOWN) {
location_t fail_loc(location);
location = LOCATION_CASE_TEARDOWN;

utest::v1::status_t teardown_status = handlers.case_teardown(case_current, case_passed, case_failed, failure_t(reason, fail_loc));
utest::v1::status_t teardown_status = handlers().case_teardown(case_current, case_passed, case_failed, failure_t(reason, fail_loc));
if (teardown_status < STATUS_CONTINUE) raise_failure(REASON_CASE_TEARDOWN);
else if (teardown_status > signed(test_length)) raise_failure(REASON_CASE_INDEX);
else if (teardown_status >= 0) case_index = teardown_status - 1;

// Restore case failure location once we have dealt with case teardown
location = fail_loc;
handlers.case_teardown = NULL;
handlers().case_teardown = NULL;
}
}
if (fail_status == STATUS_ABORT) {
test_failed++;
failure_t fail(reason, location);
location = LOCATION_TEST_TEARDOWN;
if (handlers.test_teardown) handlers.test_teardown(test_passed, test_failed, fail);
if (handlers().test_teardown) handlers().test_teardown(test_passed, test_failed, fail);
exit(test_failed);
die();
}
Expand All @@ -218,8 +225,8 @@ void Harness::schedule_next_case()
if (case_control.repeat & REPEAT_SETUP_TEARDOWN || !(case_control.repeat & (REPEAT_ON_TIMEOUT | REPEAT_ON_VALIDATE))) {
location = LOCATION_CASE_TEARDOWN;

if (handlers.case_teardown) {
utest::v1::status_t status = handlers.case_teardown(case_current, case_passed, case_failed,
if (handlers().case_teardown) {
utest::v1::status_t status = handlers().case_teardown(case_current, case_passed, case_failed,
case_failed ? failure_t(REASON_CASES, LOCATION_UNKNOWN) : failure_t(REASON_NONE));
if (status < STATUS_CONTINUE) raise_failure(REASON_CASE_TEARDOWN);
else if (status > signed(test_length)) raise_failure(REASON_CASE_INDEX);
Expand Down Expand Up @@ -298,9 +305,9 @@ void Harness::run_next_case()
UTEST_LOG_FUNCTION();
if(case_current < (test_cases + test_length))
{
handlers.case_setup = defaults.get_handler(case_current->setup_handler);
handlers.case_teardown = defaults.get_handler(case_current->teardown_handler);
handlers.case_failure = defaults.get_handler(case_current->failure_handler);
handlers().case_setup = defaults().get_handler(case_current->setup_handler);
handlers().case_teardown = defaults().get_handler(case_current->teardown_handler);
handlers().case_failure = defaults().get_handler(case_current->failure_handler);

if (case_current->is_empty()) {
location = LOCATION_UNKNOWN;
Expand All @@ -321,7 +328,7 @@ void Harness::run_next_case()

if (setup_repeat & REPEAT_SETUP_TEARDOWN) {
location = LOCATION_CASE_SETUP;
if (handlers.case_setup && (handlers.case_setup(case_current, test_index_of_case) != STATUS_CONTINUE)) {
if (handlers().case_setup && (handlers().case_setup(case_current, test_index_of_case) != STATUS_CONTINUE)) {
raise_failure(REASON_CASE_SETUP);
schedule_next_case();
return;
Expand Down Expand Up @@ -361,9 +368,9 @@ void Harness::run_next_case()
UTEST_LEAVE_CRITICAL_SECTION;
}
}
else if (handlers.test_teardown) {
else if (handlers().test_teardown) {
location = LOCATION_TEST_TEARDOWN;
handlers.test_teardown(test_passed, test_failed, test_failed ? failure_t(REASON_CASES, LOCATION_UNKNOWN) : failure_t(REASON_NONE));
handlers().test_teardown(test_passed, test_failed, test_failed ? failure_t(REASON_CASES, LOCATION_UNKNOWN) : failure_t(REASON_NONE));
test_cases = NULL;
exit(test_failed);
} else {
Expand Down
10 changes: 6 additions & 4 deletions features/frameworks/utest/source/utest_shim.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static volatile utest_v1_harness_callback_t minimal_callback;
static volatile utest_v1_harness_callback_t ticker_callback;

// Timeout object used to control the scheduling of test case callbacks
Timeout utest_timeout_object;
SingletonPtr<Timeout> utest_timeout_object;

static void ticker_handler()
{
Expand All @@ -77,7 +77,9 @@ static void ticker_handler()
static int32_t utest_us_ticker_init()
{
UTEST_LOG_FUNCTION();
// Ticker scheduler does not require any initialisation so return immediately
// initialize the Timeout object to makes sure it is not initialized in
// interrupt context.
utest_timeout_object.get();
return 0;
}
static void *utest_us_ticker_post(const utest_v1_harness_callback_t callback, timestamp_t delay_ms)
Expand All @@ -88,7 +90,7 @@ static void *utest_us_ticker_post(const utest_v1_harness_callback_t callback, ti
if (delay_ms) {
ticker_callback = callback;
// fire the interrupt in 1000us * delay_ms
utest_timeout_object.attach_us(ticker_handler, delay_us);
utest_timeout_object->attach_us(ticker_handler, delay_us);

}
else {
Expand All @@ -102,7 +104,7 @@ static int32_t utest_us_ticker_cancel(void *handle)
{
UTEST_LOG_FUNCTION();
(void) handle;
utest_timeout_object.detach();
utest_timeout_object->detach();
return 0;
}
static int32_t utest_us_ticker_run()
Expand Down
19 changes: 19 additions & 0 deletions features/frameworks/utest/source/utest_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,22 @@ const char* utest::v1::stringify(utest::v1::status_t status)
}
return "Unknown Status";
}


const utest::v1::base_control_t utest::v1::CaseNext = { REPEAT_NONE, TIMEOUT_NONE };

const utest::v1::base_control_t utest::v1::CaseNoRepeat = { REPEAT_NONE, TIMEOUT_UNDECLR };

const utest::v1::base_control_t utest::v1::CaseRepeatAll = { REPEAT_ALL, TIMEOUT_UNDECLR };

const utest::v1::base_control_t utest::v1::CaseRepeatHandler = { REPEAT_HANDLER, TIMEOUT_UNDECLR };

const utest::v1::base_control_t utest::v1::CaseNoTimeout = { REPEAT_UNDECLR, TIMEOUT_NONE };

const utest::v1::base_control_t utest::v1::CaseAwait = { REPEAT_UNDECLR, TIMEOUT_FOREVER };

// equal to CaeReapeatAll
const utest::v1::base_control_t utest::v1::CaseRepeat = { REPEAT_ALL, TIMEOUT_UNDECLR };

// equal to CaseRepeatHandler
const utest::v1::base_control_t utest::v1::CaseRepeatHandlerOnly = { REPEAT_HANDLER, TIMEOUT_UNDECLR };
2 changes: 1 addition & 1 deletion features/frameworks/utest/utest/utest_default_handlers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ namespace v1 {
extern const handlers_t selftest_handlers;

/// The greentea aborting handlers are the default
const handlers_t default_handlers = greentea_abort_handlers;
extern const handlers_t& default_handlers;

} // namespace v1
} // namespace utest
Expand Down
81 changes: 62 additions & 19 deletions features/frameworks/utest/utest/utest_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,23 @@ namespace v1 {
/// Stringifies a status.
const char* stringify(utest::v1::status_t status);

/** POD version of the class control_t.
* It is used to instantiate const control_t objects as PODs
* and prevent them to be included in the final binary.
* @note: control_pod_t can be converted to control_t by copy construction.
*/
struct base_control_t {
repeat_t repeat;
uint32_t timeout;

repeat_t inline get_repeat() const {
return repeat;
}
uint32_t inline get_timeout() const {
return timeout;
}
};

/** Control class for specifying test case attributes
*
* This class encapsulated control information about test cases which, when returned from
Expand Down Expand Up @@ -148,24 +165,26 @@ namespace v1 {
*
* In the future, more control information may be added transparently and backwards compatible.
*/
struct control_t
struct control_t : private base_control_t
{
control_t() : repeat(REPEAT_UNDECLR), timeout(TIMEOUT_UNDECLR) {}
control_t() : base_control_t(make_base_control_t(REPEAT_UNDECLR, TIMEOUT_UNDECLR)) {}

control_t(repeat_t repeat, uint32_t timeout_ms) :
repeat(repeat), timeout(timeout_ms) {}
base_control_t(make_base_control_t(repeat, timeout_ms)) {}

control_t(repeat_t repeat) :
repeat(repeat), timeout(TIMEOUT_UNDECLR) {}
base_control_t(make_base_control_t(repeat, TIMEOUT_UNDECLR)) {}

control_t(uint32_t timeout_ms) :
repeat(REPEAT_UNDECLR), timeout(timeout_ms) {}
base_control_t(make_base_control_t(REPEAT_UNDECLR, timeout_ms)) {}

control_t
inline operator+(const control_t& rhs) const {
control_t(const base_control_t& other) :
base_control_t(other) {}

friend control_t operator+(const control_t& lhs, const control_t& rhs) {
control_t result(
repeat_t(this->repeat | rhs.repeat),
(rhs.timeout == TIMEOUT_NONE) ? rhs.timeout : this->timeout);
repeat_t(lhs.repeat | rhs.repeat),
(rhs.timeout == TIMEOUT_NONE) ? rhs.timeout : lhs.timeout);

if (result.timeout != TIMEOUT_NONE && result.timeout > rhs.timeout) {
result.timeout = rhs.timeout;
Expand Down Expand Up @@ -196,25 +215,46 @@ namespace v1 {
}

private:
repeat_t repeat;
uint32_t timeout;
static base_control_t make_base_control_t(repeat_t repeat, uint32_t timeout) {
base_control_t result = {
repeat,
timeout
};
return result;
}

friend class Harness;
};

/// @see operator+ in control_t
inline control_t operator+(const base_control_t& lhs, const base_control_t& rhs) {
return control_t(lhs) + control_t(rhs);
}

/// @see operator+ in control_t
inline control_t operator+(const base_control_t& lhs, const control_t& rhs) {
return control_t(lhs) + rhs;
}

/// @see operator+ in control_t
inline control_t operator+(const control_t& lhs, const base_control_t& rhs) {
return lhs + control_t(rhs);
}

/// does not repeat this test case and immediately moves on to the next one without timeout
const control_t CaseNext(REPEAT_NONE, TIMEOUT_NONE);
extern const base_control_t CaseNext;

/// does not repeat this test case, moves on to the next one
const control_t CaseNoRepeat(REPEAT_NONE);
extern const base_control_t CaseNoRepeat;
/// repeats the test case handler with calling teardown and setup handlers
const control_t CaseRepeatAll(REPEAT_ALL);
extern const base_control_t CaseRepeatAll;
/// repeats only the test case handler without calling teardown and setup handlers
const control_t CaseRepeatHandler(REPEAT_HANDLER);
extern const base_control_t CaseRepeatHandler;

/// No timeout, immediately moves on to the next case, but allows repeats
const control_t CaseNoTimeout(TIMEOUT_NONE);
extern const base_control_t CaseNoTimeout;
/// Awaits until the callback is validated and never times out. Use with caution!
const control_t CaseAwait(TIMEOUT_FOREVER);
extern const base_control_t CaseAwait;
/// Alias class for asynchronous timeout control in milliseconds
inline control_t CaseTimeout(uint32_t ms) { return ms; }

Expand Down Expand Up @@ -341,8 +381,11 @@ namespace v1 {


// deprecations
__deprecated_message("Use CaseRepeatAll instead.") const control_t CaseRepeat = CaseRepeatAll;
__deprecated_message("Use CaseRepeatHandler instead.") const control_t CaseRepeatHandlerOnly = CaseRepeatHandler;
__deprecated_message("Use CaseRepeatAll instead.")
extern const base_control_t CaseRepeat;
Copy link
Contributor

@geky geky Aug 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a different pr, but should we update these to use MBED_DEPRECATED instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different PR, but yeah totally :)


__deprecated_message("Use CaseRepeatHandler instead.")
extern const base_control_t CaseRepeatHandlerOnly;

__deprecated_message("Use REASON_NONE instead.") const failure_reason_t FAILURE_NONE = REASON_NONE;
__deprecated_message("Use REASON_UNKNOWN instead.") const failure_reason_t FAILURE_UNKNOWN = REASON_UNKNOWN;
Expand Down