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

Split EkatCreateUnitTest in two macros #166

Merged
merged 2 commits into from
Nov 23, 2021

Conversation

bartgol
Copy link
Contributor

@bartgol bartgol commented Nov 22, 2021

Motivation

A recent change made it possible to create a unit test with an existing executable. However, the user still had to pass an empty argument for the exec sources. This PR fixes this, by splitting the existing macro into two macros: one to create the executable (EkatCreateUnitTestExec), and the other to create the tests (EkatCreateUnitTestFromExec). The old macro EkatCreateUnitTest is still available (but does not support the TEST_EXE optional arg anymore), and simply calls the other two in sequence (plus some logic to split the optional arg among the two macros).

To create structurally different tests with the same exec, one can now do

EkatCreateUnitTestExec(my_exe src1 [src2..] <options_for_building_exec> )

EkatCreateUnitTestFromExec (test1 my_exe <options_for_test1>)
EkatCreateUnitTestFromExec (test2 my_exe <options_for_test2>)

or even

EkatCreateUnitTest (test1 src1 [src2] <options_for_building_exec> <options_for_test1>)
EkatCreateUnitTestFromExec (test2 test1 <options_for_test2>)

(EkatCreateUnitTest makes the exec name coincide with the root test name).

Testing

I manually verified that the two macros work as expected. All existing EKAT tests use the old macro, which is now implemented in terms of the two new ones, making them "tested".

One macro creates the exec, the other creates the tests.
The old macro EkatCreateUnitTest is still available,
and simply calls the two new macros (plus some logic to
split the optional args).
@bartgol bartgol added cmake AT: AUTOMERGE Inform the autotester (AT) that it can merge this PR if reviewers approved, and tests pass labels Nov 22, 2021
@bartgol bartgol requested a review from jgfouca November 22, 2021 23:45
@bartgol bartgol self-assigned this Nov 22, 2021
@bartgol
Copy link
Contributor Author

bartgol commented Nov 22, 2021

Note: the old macro could have been implemented easily as

macro (EkatCreateUnitTest test_name test_srcs)
  EkatCreateUnitTestExec (${test_name} ${test_srcs} ${ARGN})
  EkatCreateUnitTestFromExec (${test_name} ${test_name} ${ARGN})
endmacro ()

However, this would cause lots of cmake warning from the inner macros, which would receive many more args than what they expect (they would both receive both add-exec and add-test phase args). I could have dropped the check on the input args, but it seemed unsafe, so I had to add some logic to split the input lists of optional args into lists for exec-creation and tests-creation phases.

@E3SM-Autotester
Copy link
Collaborator

Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request.

@E3SM-Autotester
Copy link
Collaborator

Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects:

Pull Request Auto Testing STARTING (click to expand)

Build Information

Test Name: EKAT_PullRequest_Autotester_Blake

  • Build Num: 284
  • Status: STARTED

Jenkins Parameters

Parameter Name Value
EKAT_SOURCE_BRANCH bartgol/create-unit-test-macros
EKAT_SOURCE_REPO https://github.com/E3SM-Project/EKAT
EKAT_SOURCE_SHA 3c1cf1d
EKAT_TARGET_BRANCH master
EKAT_TARGET_REPO https://github.com/E3SM-Project/EKAT
EKAT_TARGET_SHA 7f7ab1b
PR_LABELS cmake;AT: AUTOMERGE
PULLREQUESTNUM 166
TEST_REPO_ALIAS EKAT

Build Information

Test Name: EKAT_PullRequest_Autotester_Weaver

  • Build Num: 275
  • Status: STARTED

Jenkins Parameters

Parameter Name Value
EKAT_SOURCE_BRANCH bartgol/create-unit-test-macros
EKAT_SOURCE_REPO https://github.com/E3SM-Project/EKAT
EKAT_SOURCE_SHA 3c1cf1d
EKAT_TARGET_BRANCH master
EKAT_TARGET_REPO https://github.com/E3SM-Project/EKAT
EKAT_TARGET_SHA 7f7ab1b
PR_LABELS cmake;AT: AUTOMERGE
PULLREQUESTNUM 166
TEST_REPO_ALIAS EKAT

Build Information

Test Name: EKAT_PullRequest_Autotester_Mappy

  • Build Num: 179
  • Status: STARTED

Jenkins Parameters

Parameter Name Value
EKAT_SOURCE_BRANCH bartgol/create-unit-test-macros
EKAT_SOURCE_REPO https://github.com/E3SM-Project/EKAT
EKAT_SOURCE_SHA 3c1cf1d
EKAT_TARGET_BRANCH master
EKAT_TARGET_REPO https://github.com/E3SM-Project/EKAT
EKAT_TARGET_SHA 7f7ab1b
PR_LABELS cmake;AT: AUTOMERGE
PULLREQUESTNUM 166
TEST_REPO_ALIAS EKAT

Using Repos:

Repo: EKAT (E3SM-Project/EKAT)
  • Branch: bartgol/create-unit-test-macros
  • SHA: 3c1cf1d
  • Mode: TEST_REPO

Pull Request Author: bartgol

@E3SM-Autotester
Copy link
Collaborator

Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED

Pull Request Auto Testing has PASSED (click to expand)

Build Information

Test Name: EKAT_PullRequest_Autotester_Blake

  • Build Num: 284
  • Status: PASSED

Jenkins Parameters

Parameter Name Value
EKAT_SOURCE_BRANCH bartgol/create-unit-test-macros
EKAT_SOURCE_REPO https://github.com/E3SM-Project/EKAT
EKAT_SOURCE_SHA 3c1cf1d
EKAT_TARGET_BRANCH master
EKAT_TARGET_REPO https://github.com/E3SM-Project/EKAT
EKAT_TARGET_SHA 7f7ab1b
PR_LABELS cmake;AT: AUTOMERGE
PULLREQUESTNUM 166
TEST_REPO_ALIAS EKAT

Build Information

Test Name: EKAT_PullRequest_Autotester_Weaver

  • Build Num: 275
  • Status: PASSED

Jenkins Parameters

Parameter Name Value
EKAT_SOURCE_BRANCH bartgol/create-unit-test-macros
EKAT_SOURCE_REPO https://github.com/E3SM-Project/EKAT
EKAT_SOURCE_SHA 3c1cf1d
EKAT_TARGET_BRANCH master
EKAT_TARGET_REPO https://github.com/E3SM-Project/EKAT
EKAT_TARGET_SHA 7f7ab1b
PR_LABELS cmake;AT: AUTOMERGE
PULLREQUESTNUM 166
TEST_REPO_ALIAS EKAT

Build Information

Test Name: EKAT_PullRequest_Autotester_Mappy

  • Build Num: 179
  • Status: PASSED

Jenkins Parameters

Parameter Name Value
EKAT_SOURCE_BRANCH bartgol/create-unit-test-macros
EKAT_SOURCE_REPO https://github.com/E3SM-Project/EKAT
EKAT_SOURCE_SHA 3c1cf1d
EKAT_TARGET_BRANCH master
EKAT_TARGET_REPO https://github.com/E3SM-Project/EKAT
EKAT_TARGET_SHA 7f7ab1b
PR_LABELS cmake;AT: AUTOMERGE
PULLREQUESTNUM 166
TEST_REPO_ALIAS EKAT

@E3SM-Autotester
Copy link
Collaborator

Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ jgfouca ]!

@E3SM-Autotester
Copy link
Collaborator

Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged

@E3SM-Autotester E3SM-Autotester merged commit 17bb2da into master Nov 23, 2021
@E3SM-Autotester E3SM-Autotester deleted the bartgol/create-unit-test-macros branch November 23, 2021 01:30
@E3SM-Autotester
Copy link
Collaborator

Merge on Pull Request# 166: IS A SUCCESS - Pull Request successfully merged

@E3SM-Autotester E3SM-Autotester removed the AT: AUTOMERGE Inform the autotester (AT) that it can merge this PR if reviewers approved, and tests pass label Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants