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.

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

Fix common test framework options #10975

Closed
wants to merge 1 commit into from

Conversation

mattcaswell
Copy link
Member

@mattcaswell mattcaswell commented Jan 30, 2020

PR #6975 added the ability to our test framework to have common options to
all tests. For example providing the option "-test 5" to one of our test
programs will just run test number 5. This can be useful when debugging
tests.

Unforuntately this does not work well for a number of tests. In particular
those test that call test_get_argument() without first skipping over these
common test options will not got the expected value. Some tests did this
correctly but a large number did not.

A helper function is introduced, test_skip_common_options(), to make this
easier for those tests which do not have their own specialised test option
handling, but yet still need to call test_get_argument(). This function
call is then added to all those tests that need it.

Checklist
  • documentation is added or updated
  • tests are added or updated

@mattcaswell mattcaswell added the branch: master Merge to master branch label Jan 30, 2020
@mspncp
Copy link
Contributor

mspncp commented Jan 30, 2020

Two typos in commit message:

In particular
those tests that call test_get_argument() without first skipping over these
common test options will not get the expected value.

PR#6975 added the ability to our test framework to have common options to
all tests. For example providing the option "-test 5" to one of our test
programs will just run test number 5. This can be useful when debugging
tests.

Unforuntately this does not work well for a number of tests. In particular
those tests that call test_get_argument() without first skipping over these
common test options will not get the expected value. Some tests did this
correctly but a large number did not.

A helper function is introduced, test_skip_common_options(), to make this
easier for those tests which do not have their own specialised test option
handling, but yet still need to call test_get_argument(). This function
call is then added to all those tests that need it.
@mattcaswell
Copy link
Member Author

Commit message updated to fix the typos.

Copy link
Member

@slontis slontis left a comment

Choose a reason for hiding this comment

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

Good work..
I had noticed this bug - just didnt get back to address it.

@slontis slontis added the approval: done This pull request has the required number of approvals label Jan 30, 2020
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

Nice change.

I'd planned on doing this automatically in the test harness before running the specific test code which would have reduced the copy/paste duplication. Still, far better to have it in and working than only having it as an idea.

@mattcaswell
Copy link
Member Author

Pushed. Thanks.

@mattcaswell mattcaswell closed this Feb 3, 2020
openssl-machine pushed a commit that referenced this pull request Feb 3, 2020
PR#6975 added the ability to our test framework to have common options to
all tests. For example providing the option "-test 5" to one of our test
programs will just run test number 5. This can be useful when debugging
tests.

Unforuntately this does not work well for a number of tests. In particular
those tests that call test_get_argument() without first skipping over these
common test options will not get the expected value. Some tests did this
correctly but a large number did not.

A helper function is introduced, test_skip_common_options(), to make this
easier for those tests which do not have their own specialised test option
handling, but yet still need to call test_get_argument(). This function
call is then added to all those tests that need it.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #10975)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants