Skip to content

Problems with compare_local_time unit test #3027

@billsacks

Description

@billsacks

In my work on #3017 , I noticed some problems with the compare_local_time unit test in src/utils/test/clm_time_manager_test/test_clm_time_manager.pf:

  1. This test currently isn't doing anything because the start time (with secs = 0) is at the end of a year, so the do while ( .not. is_end_curr_year() ) is never entered. For this test to do anything, it would need to start with, e.g., secs = 1800.
  2. When I start with secs = 1800 so the test does something, it fails the assertion (I think in the first loop iteration, but I'm not sure)
  3. The loop over longitudes goes to too high of a longitude: it loops up to longitude = 360, but then increments the longitude before calling the functions, so attempts to call the functions with longitude = 360.1, which causes an abort
  4. If I fix issues (1) and (3) and get rid of the assertion - just storing the results of the two calls - then this test takes a really long time: Without this test, all of the tests of clm_time_manager take about 0.05 seconds. With this test, this goes up to about 100 seconds on my Mac.

I find unit tests most useful when they test a few targeted cases. It means they may cover fewer inputs, but they will run faster and (importantly to me) more clearly communicate any special cases or conditions that need to be considered with the code under test. And then, if there's a failure, it's more clear what conditions pass and what conditions fail. So my suggestion would be to think of a few specific cases to test here rather than having a doubly nested loop which currently (I believe) tests 63072000 cases.

Metadata

Metadata

Assignees

Labels

bfbbit-for-bitpriority: lowBackground task that doesn't need to be done right away.testingadditions or changes to tests

Type

Projects

Status

Todo

Status

No status

Relationships

None yet

Development

No branches or pull requests

Issue actions