Skip to content

utest optimization: Allow case data structure to be put in ROM. #4430

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

Jump to bottom
Merged
merged 3 commits into from
Jun 7, 2017
Merged

utest optimization: Allow case data structure to be put in ROM. #4430

merged 3 commits into from
Jun 7, 2017

Conversation

pan-
Copy link
Member

@pan- pan- commented Jun 2, 2017

Description

This patch split the Case class in two entities: Case and case_t. case_t contains the test case data structure while Case provide the interface to the test case. Unlike the class Case, case _t is a POD and can be instantiated at compile time and put in the constant data section (in ROM).

The Specification class has also been modified to accept arrays of case_t.

It fixes #4363 and will allow tests with many cases on the most constrained targets.

Status

READY

Migrations

NO

@pan-
Copy link
Member Author

pan- commented Jun 2, 2017 •
edited
Loading

@0xc0170 @adbridge Could you review this PR.

@pan- pan- mentioned this pull request Jun 2, 2017
Merged
theotherjimmy
theotherjimmy reviewed Jun 2, 2017
View reviewed changes
features/frameworks/utest/utest/utest_case.h Outdated

const case_failure_handler_t failure_handler;

// see data member in case_t
Copy link
Contributor

Choose a reason for hiding this comment

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

what member? I don't see a member named data.

Copy link
Member Author

Choose a reason for hiding this comment

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

I should rephrase the comment and makes it more explicit.
This class shall not declare data members.

Data members are declared in case_t and imported via private inheritance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification!

theotherjimmy
theotherjimmy approved these changes Jun 2, 2017
View reviewed changes
Copy link
Contributor

@theotherjimmy theotherjimmy left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification!

sg-
sg- reviewed Jun 3, 2017
View reviewed changes
features/frameworks/utest/utest/utest_case.h Outdated
@@ -164,7 +164,10 @@ namespace v1 {
bool is_empty() const;

private:
// see data member in case_t
// IMPORTANT: No data members shall be declared inside this class.
// Data members shall be declared in case_t to preserve the layout
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: spacing seems off by 1

@sg- sg- added the needs: CI label Jun 3, 2017
@pan-
Copy link
Member Author

pan- commented Jun 5, 2017

@sg- I've fixed the indentation of the comment.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Jun 5, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 440

Build Prep failed!

@adbridge
Copy link
Contributor

adbridge commented Jun 5, 2017

@studavekar If I've tracked the build node failure through correctly we have this:

04:53:15  > git checkout -f 40042f0b000e32d94fa619edf8f21cbd3d86bb73
05:03:15 ERROR: Timeout after 10 minutes
05:03:25 FATAL: Could not checkout 40042f0b000e32d94fa619edf8f21cbd3d86bb73
05:03:48 hudson.plugins.git.GitException: Command "git checkout -f 40042f0b000e32d94fa619edf8f21cbd3d86bb73" returned status code -1:
05:03:48 stdout: Process leaked file descriptors. See http://wiki.jenkins-ci.org/display/JENKINS/Spawning+processes+from+build for more information
05:03:48 Process leaked file descriptors. See http://wiki.jenkins-ci.org/display/JENKINS/Spawning+processes+from+build for more information

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017

One more time

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017 •
edited
Loading

@pan- Can you rebase on top of master? to fix jenkins CI. I should have not run morph here, let me try to abort this one

@mbed-bot
Copy link

mbed-bot commented Jun 5, 2017

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

pan- added 3 commits June 5, 2017 16:06
@pan-
utest optimization: Allow case data structure to be put in ROM.
5b27d65 
This patch split the Case class in two entities: Case and case_t. case_t contains the test case data structure while Case provide the interface to the test case. Unlike the class Case, case _t is a POD and can be instantiated at compile time and put in the constant data section (in ROM).

The Specification class has also been modified to accept arrays of case_t.
@pan-
utest case: Indicate explicitely that no data member shall be declare…
165d76a 
…d in

the Case class.
@pan-
utest_case.h : Fix comment indentation
f36619b
@pan- pan- force-pushed the utest_cases_romable branch from 207949f to f36619b Compare June 5, 2017 15:07
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Jun 5, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 449

All builds and test passed!

@sg- sg- merged commit 28f590f into ARMmbed:master Jun 7, 2017
@pan- pan- mentioned this pull request Jun 7, 2017
Merged
@pan- pan- deleted the utest_cases_romable branch November 14, 2018 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sg- sg- sg- left review comments

@theotherjimmy theotherjimmy theotherjimmy approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

Assignees
No one assigned
Labels
release-version: 5.5.0-rc2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

utest: Description data structures are not ROMable
6 participants
@pan- @0xc0170 @mbed-bot @adbridge @sg- @theotherjimmy