test.sh: Enable running by named-function w/o shared memory.

gapisback commented 7 months ago

This commit adds minor improvements / bug-fixes to test.sh:

The capability to run individual test-function(s) by name was only working with the "--use-shmem" flag but was not working when test.sh is invoked with just the test-execution-fn-name. We would get a bash parameter parsing error.

Rework the parameter parsing in all test-functions so that we can now invoke this driver script as:

$ INCLUDE_SLOW_TESTS=true ./test.sh run_slower_unit_tests

This will run individually named test-functions with default memory configuration.
run_other_driver_tests() had a bug where tests run by this function were not honoring '--use-shmem' arg. Fix this so that cache_test, log_test, filter_test can now also be run with "--use-shmem" enabled.
Update elapsed-time tracking to separately track the test execution run-times w/o and w/ shared memory configured.
Other minor cleanup of comments, usage() info etc.

--- NOTE ---

In main, without this fix, the following invocation will result in a bash-script parsing error:

cc-sdb-vm:[130] $ INCLUDE_SLOW_TESTS=true ./test.sh run_slower_unit_tests

test.sh: build_dir='build/release', BINDIR='build/release/bin'
test.sh: Sun Dec 10 13:24:29 PST 2023 Start SplinterDB Test Suite Execution.
+ SEED=135
+ run_type=' '
+ '[' false == true ']'
+ set +x
./test.sh: line 628: $1: unbound variable

With this fix, this usage will now be supported and will run the named test-execution function, run_slower_unit_tests(), without the use of shared memory configuration.

The improved test summary output can be seen in any of the CI jobs run for this PR. For example, see this one:

Sun Dec 10 21:28:25 America 2023 **** SplinterDB Test Suite Execution Times **** 

Fast unit tests                                                                      :  105s [  0h  1m 45s ]
[...]
Filter test                                                                          :    3s [  0h  0m  3s ]
Tests without shared memory configured                                               : 1269s [  0h 21m  9s ]

-- Tests with shared memory configured --

Fast unit tests using shared memory                                                  :  121s [  0h  2m  1s ]
[...]

Splinter large inserts test using shared memory, 1 forked child                      :    2s [  0h  0m  2s ]
Tests with shared memory configured                                                  : 1536s [  0h 25m 36s ]
All Tests                                                                            : 2805s [  0h 46m 45s ]

netlify[bot] commented 7 months ago

Deploy Preview for splinterdb canceled.

Name	Link
Latest commit	718574a99d26242c38ddbf816e376717d1865be7
Latest deploy log	https://app.netlify.com/sites/splinterdb/deploys/65b021d6318f720007eba7e7

gapisback commented 7 months ago

@rtjohnso -- As these are simple corrections to test.sh, I am requesting @rosenhouse for a review, that is assuming Rob is busy and may be Gabriel has a few mins to spare.

I'm saving Rob's efforts for upcoming more code-related PR reviews.

gapisback commented 7 months ago

hi, @rtjohnso -- Sorry, I had to bounce this review back to you. It's minor rework of logic in test.sh ... And I couldn't get anyone else to look at it.

Thanks!

gapisback commented 6 months ago

@rtjohnso : Re: Any reason the use_shmem pattern you noted can't be factored out into a top-level argument parsing step?

The chunks of repeated code did, also, bother me even while I was re-working test.sh. The problem is that in this usage, for example:

 607 function run_fast_unit_tests() {
 608     local use_shmem=""
 609     if [ $# -eq 1 ]; then
 610         use_shmem="$1"
 611    fi

The outer test.sh can be executed in one of two ways, where this code comes into play, e.g.,:

a) Using default memory configuration: $INCLUDE_SLOW_TESTS=true ./test.sh run_fast_unit_tests

b) Using shared-memory configuration: $INCLUDE_SLOW_TESTS=true ./test.sh run_fast_unit_tests --use-shmem

Upon your nudge, I decided refactor this into a top-level parsing step saving off the --use-shmem arg in a global,Use_shmem variable.

I have reworked the change set appropriately and have dev unit-tested different sub-test-function commands to ensure that this new logic will still work as expected. It seems to work out ok.

CI tests are running now ... and once they clear, I will merge this PR into /main.

vmware / splinterdb