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 ctests so each restart test has a seed unique to the first test #1832

Merged
merged 4 commits into from
Jul 16, 2019

Conversation

sfc-gh-dyoungworth
Copy link
Collaborator

Makes sure that the ctest infastructure generates a unique seed for every test run, including the first and second tests of each restart test.

Also protect against potential bug where first restart test's logs could be overwritten by the second. (Only happens if logs files do not generate a unique id)

@sfc-gh-dyoungworth
Copy link
Collaborator Author

sfc-gh-dyoungworth commented Jul 11, 2019

Ran a partial ctest suite to make sure each test run has a unique seed:

[dyoungworth@ceta-fedora28 FDBA3]$ ctest -R 'RandomReadWriteTest|SwizzledLargeApiCorrectness|restarting'
Test project /home/dyoungworth/builds/FDBA3
Start 53: rare/RandomReadWriteTest
1/9 Test #53: rare/RandomReadWriteTest ........................ Passed 40.60 sec
Start 54: rare/SwizzledLargeApiCorrectness
2/9 Test #54: rare/SwizzledLargeApiCorrectness ................ Passed 39.69 sec
Start 55: restarting/ConfigureTestRestart-1
3/9 Test #55: restarting/ConfigureTestRestart-1 ............... Passed 27.02 sec
Start 56: restarting/CycleTestRestart-1
4/9 Test #56: restarting/CycleTestRestart-1 ................... Passed 20.91 sec
Start 57: restarting/StorefrontTestRestart-1
5/9 Test #57: restarting/StorefrontTestRestart-1 .............. Passed 44.67 sec
Start 58: restarting/from_6.2.0/SnapTestSimpleRestart-1
6/9 Test #58: restarting/from_6.2.0/SnapTestSimpleRestart-1 ... Passed 7.68 sec
Start 59: restarting/from_6.2.0/SnapTestRestart-1

[dyoungworth@ceta-fedora28 FDBA3]$ grep RandomSeed test_runs/2019_07_11__09_48_41/test_/.json | awk '{print $12} {print $13} {print $25}'
"RandomSeed":
"12398",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_rare_RandomReadWriteTest",
"RandomSeed":
"12399",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_rare_SwizzledLargeApiCorrectness",
"RandomSeed":
"12400",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_ConfigureTestRestart-1",
"RandomSeed":
"12401",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_ConfigureTestRestart-1",
"RandomSeed":
"12402",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_CycleTestRestart-1",
"RandomSeed":
"12403",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_CycleTestRestart-1",
"RandomSeed":
"12408",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_from_6.2.0_SnapTestRestart-1",
"RandomSeed":
"12406",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_from_6.2.0_SnapTestSimpleRestart-1",
"RandomSeed":
"12407",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_from_6.2.0_SnapTestSimpleRestart-1",
"RandomSeed":
"12404",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_StorefrontTestRestart-1",
"RandomSeed":
"12405",
"/home/dyoungworth/builds/FDBA3/test_runs/2019_07_11__09_48_41/test_restarting_StorefrontTestRestart-1",

@@ -91,6 +91,9 @@ function(add_fdb_test)
# if the value was undefined before, it will still be
# undefined.
math(EXPR assigned_id "${test_idx} - 1")
math(EXPR test_file_idx "${CURRENT_TEST_FILE_INDEX} + ${NUM_TEST_FILES}")
Copy link
Contributor

Choose a reason for hiding this comment

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

the only thing you should change here is the math-expression for test_idx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do. I kept the old test_idx just so when printing tests you get a contiguous count, but that doesn't happen when tests are skipped anyway

@@ -91,6 +91,9 @@ function(add_fdb_test)
# if the value was undefined before, it will still be
# undefined.
math(EXPR assigned_id "${test_idx} - 1")
math(EXPR test_file_idx "${CURRENT_TEST_FILE_INDEX} + ${NUM_TEST_FILES}")
set(CURRENT_TEST_FILE_INDEX "${test_file_idx}" PARENT_SCOPE)
math(EXPR assigned_test_number "${test_file_idx} - ${NUM_TEST_FILES}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Basically same as above: instead of introducing a new variable, just change the math for assigned_id

@hgray1 hgray1 added this to the 6.2 milestone Jul 15, 2019
@etschannen etschannen merged commit 2520b69 into apple:master Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants