Merge pull request #1771 from billsacks/unittest_throw_exception

Facilitate expected error unit tests
Pull shr_sys_abort into its own module - now call shr_abort_abort in
shr_abort_mod. (The routine name, while redundant, follows the share
code convention of MODULENAME_VERB.) Other code can still use this via
shr_sys_mod: shr_sys_abort, via a rename done in that module. Two other
routines from shr_sys_abort that are used by shr_abort_abort are also
moved into shr_abort_abort to avoid circular dependencies.

There are no substantive changes in shr_abort_mod relative to the code
that used to be in shr_sys_abort: I have just moved code into this new
module and done some reformatting.

The purpose of this separation was to facilitate using a replacement
abort routine in pFUnit-based unit tests. This PR also adds a module
thath provides that replacement, which throws a pFUnit exception rather
than actually aborting. This also changes the cime unit test build so
that this new pFUnit-based abort routine is included in all drv unit
tests.

This also adds a simple unit test of the pFUnit-based abort
routine. This demonstrates how you can use this to write your own
"expected failure" unit tests:

  @test
  subroutine test_abort(this)
    class(TestShrAbort), intent(inout) :: this

    call shr_abort_abort('Test message')
    @assertExceptionRaised('ABORTED: Test message')
  end subroutine test_abort
Test suite:

scripts_regression_tests on yellowstone and cheyenne
cime fortran unit tests with gnu on my Mac
SMS.f09_g16.X and SMS_D.f09_g16.X with:
hobart_nag
hobart_pgi
yellowstone_intel
cheyenne_gnu
SMS.f09_g16.X and SMS_D.f09_g16.X with cheyenne_intel, with a
shr_sys_abort call inserted: made sure runs still die properly when
they call shr_sys_abort
Test baseline: N/A
Test namelist changes: none
Test status: bit for bit
Fixes #140

User interface changes?: none

Update gh-pages html (Y/N)?: N

Code review:

@ekluzek @bandre-ucar - this could be useful for adding expected error
unit tests in CLM

CMakeLists.txt

@@ -84,6 +84,7 @@ endif()
 # by different tests?)
 set(SHARE_ROOT "${CIME_ROOT}/src/share")
 add_subdirectory(${SHARE_ROOT}/util csm_share)
+add_subdirectory(${SHARE_ROOT}/unit_test_stubs/util csm_share_stubs)
 include_directories(${SHARE_ROOT}/include)
 
 # esmf_wrf_timemgr not built here because it depends on csm_share.

src/externals/CMake/CIME_utils.cmake

@@ -72,6 +72,10 @@ find_package(pFUnit)
 # Preprocessor and driver handling.
 include(pFUnit_utils)
 
+# Need to add PFUNIT_INCLUDE_DIRS to the general list of include_directories
+# because we use pfunit's 'throw'.
+include_directories("${PFUNIT_INCLUDE_DIRS}")
+
 #=================================================
 # Source list and path utilities.
 #=================================================

src/share/test/unit/CMakeLists.txt

@@ -1,5 +1,7 @@
 add_subdirectory(mock)
 
+add_subdirectory(shr_abort_test)
+
 add_subdirectory(shr_assert_test)
 
 add_subdirectory(shr_spfn_test)

src/share/test/unit/mock/README

@@ -0,0 +1,4 @@
+This directory contains mock/stub modules that are just built as part of select
+unit tests of the share code. This directory should *NOT* generally be added by
+unit tests of other components.
+

src/share/test/unit/mock/shr_sys_mod.nompi_abortthrows.F90

@@ -7,11 +7,17 @@ module shr_sys_mod
 use shr_kind_mod, only: &
      shr_kind_in, shr_kind_r8
 
+use shr_abort_mod, only : shr_sys_abort => shr_abort_abort
+
 implicit none
 private
 save
 
 ! Fake abort
+! Imported from shr_abort_mod and republished with renames. Other code that wishes to use
+! these routines should use these shr_sys names rather than directly using the routines
+! from shr_abort_abort. (This is for consistency with older code, from when these routines
+! were defined in shr_sys_mod.)
 public :: shr_sys_abort
 
 ! Fake sleep
@@ -22,19 +28,6 @@ module shr_sys_mod
 
 contains
 
-subroutine shr_sys_abort(string, rc)
-  use pfunit_mod, only: throw
-
-  character(*), optional :: string
-  integer(shr_kind_in), optional :: rc
-
-  ! Prevent compiler spam about unused variables.
-  if (.false.) rc = rc
-
-  call throw("ABORTED: "//trim(string))
-
-end subroutine shr_sys_abort
-
 subroutine shr_sys_sleep(sec)
   real(shr_kind_r8), intent(in) :: sec
 

src/share/test/unit/shr_abort_test/CMakeLists.txt

@@ -0,0 +1,13 @@
+# Local pFUnit files.
+set(pf_sources
+  test_shr_abort.pf)
+
+# Sources to test.
+set(sources_needed
+  shr_kind_mod.F90
+  shr_abort_mod.abortthrows.F90)
+extract_sources("${sources_needed}" "${share_sources}" test_sources)
+
+# Do source preprocessing and add the executable.
+create_pFUnit_test(shr_abort_mod shr_abort_mod_exe "${pf_sources}"
+  "${test_sources}")

src/share/test/unit/shr_abort_test/README

@@ -0,0 +1,2 @@
+This directory tests the version of shr_abort_mod that is used in unit tests. It
+does NOT test the production version of shr_abort_mod.

src/share/test/unit/shr_abort_test/test_shr_abort.pf

@@ -0,0 +1,39 @@
+module test_shr_abort
+
+  ! Tests of shr_abort_mod: version used in unit tests that throws a pfunit exception
+  ! rather than aborting
+
+  use pfunit_mod
+  use shr_abort_mod
+  use shr_kind_mod , only : r8 => shr_kind_r8
+
+  implicit none
+
+  @TestCase
+  type, extends(TestCase) :: TestShrAbort
+   contains
+     procedure :: setUp
+     procedure :: tearDown
+  end type TestShrAbort
+
+  real(r8), parameter :: tol = 1.e-13_r8
+
+contains
+
+  subroutine setUp(this)
+    class(TestShrAbort), intent(inout) :: this
+  end subroutine setUp
+
+  subroutine tearDown(this)
+    class(TestShrAbort), intent(inout) :: this
+  end subroutine tearDown
+
+  @Test
+  subroutine test_abort(this)
+    class(TestShrAbort), intent(inout) :: this
+
+    call shr_abort_abort('Test message')
+    @assertExceptionRaised('ABORTED: Test message')
+  end subroutine test_abort
+
+end module test_shr_abort

src/share/test/unit/shr_assert_test/CMakeLists.txt

@@ -3,7 +3,8 @@ set(pf_sources test_assert.pf test_assert_array.pf test_macro.pf
 
 set(sources_needed shr_kind_mod.F90 shr_infnan_mod.F90
   shr_strconvert_mod.F90 shr_log_mod.F90
-  shr_sys_mod.nompi_abortthrows.F90 shr_assert_mod.F90)
+  shr_sys_mod.nompi_abortthrows.F90 shr_abort_mod.abortthrows.F90
+  shr_assert_mod.F90)
 
 extract_sources("${sources_needed}" "${share_sources}" test_sources)
 

src/share/test/unit/shr_spfn_test/CMakeLists.txt

@@ -3,7 +3,8 @@ set(pf_sources test_erf_r4.pf test_erf_r8.pf test_gamma_factorial.pf
 
 set(sources_needed shr_kind_mod.F90 shr_const_mod.F90 shr_infnan_mod.F90
   shr_strconvert_mod.F90 shr_log_mod.F90
-  shr_sys_mod.nompi_abortthrows.F90 shr_spfn_mod.F90)
+  shr_sys_mod.nompi_abortthrows.F90 shr_abort_mod.abortthrows.F90
+  shr_spfn_mod.F90)
 
 extract_sources("${sources_needed}" "${share_sources}" test_sources)
 

src/share/test/unit/shr_string_test/CMakeLists.txt

@@ -10,6 +10,7 @@ set(sources_needed
   shr_log_mod.F90
   shr_strconvert_mod.F90
   shr_sys_mod.nompi_abortthrows.F90
+  shr_abort_mod.abortthrows.F90
   shr_timer_mod.F90)
 
 extract_sources("${sources_needed}" "${share_sources}" test_sources)

src/share/unit_test_stubs/README

@@ -1,2 +1,2 @@
-This directory contains stubs that may be useful for the unit test builds for a
-number of components.
+This directory contains stubs and other unit test-specific implementations that
+may be useful for the unit test builds for a number of components.

src/share/unit_test_stubs/util/CMakeLists.txt

@@ -0,0 +1,4 @@
+list(APPEND share_sources
+  shr_abort_mod.abortthrows.F90)
+
+sourcelist_to_parent(share_sources)

src/share/unit_test_stubs/util/README

@@ -0,0 +1,3 @@
+This directory contains some fake modules that should generally be used in place
+of their real counterparts by ALL unit tests. So this directory generally should
+be added by unit tests of other components.

src/share/unit_test_stubs/util/shr_abort_mod.abortthrows.F90

@@ -0,0 +1,63 @@
+module shr_abort_mod
+
+  ! This is a replacement for shr_abort_mod that throws a pfunit exception rather than
+  ! aborting
+
+  use shr_kind_mod, only : shr_kind_in
+  use pfunit_mod, only : throw
+
+  implicit none
+  private
+
+  public :: shr_abort_abort ! Replacement for shr_abort_abort that throws a pfunit exception rather than aborting
+
+  public :: shr_abort_backtrace ! Just to satisfy the public interface of shr_abort_abort
+
+contains
+
+  subroutine shr_abort_abort(string,rc)
+    ! Replacement for shr_abort_abort that throws a pfunit exception rather than aborting
+    !
+    ! This can be used to test expected errors (i.e., failure testing).
+    !
+    ! If this occurs within a pFUnit-based test:
+    !
+    ! - If you have code like:
+    !
+    !   @assertExceptionRaised(expected_message)
+    !
+    !   then your test will pass if the actual message in the 'throw' call (including the
+    !   'ABORTED: ' prefix) matches expected_message; it will fail if the actual message
+    !   doesn't match the expected message
+    !
+    ! - If you don't have
+    !
+    !   @assertExceptionRaised
+    !
+    !   or
+    !
+    !   call assertExceptionRaised
+    !
+    !   then this will result in the given pFUnit test failing.
+
+    !----- arguments -----
+    character(len=*)    , intent(in), optional :: string  ! error message string
+    integer(shr_kind_in), intent(in), optional :: rc      ! error code
+
+    !----- locals -----
+    integer(shr_kind_in) :: my_rc
+
+    ! Prevent compiler spam about unused variables.
+    if (.false.) my_rc = rc
+
+    call throw("ABORTED: "//trim(string))
+  end subroutine shr_abort_abort
+
+  subroutine shr_abort_backtrace()
+    ! Just to satisfy the public interface of shr_abort_abort
+    !
+    ! Does nothing
+
+  end subroutine shr_abort_backtrace
+
+end module shr_abort_mod

src/share/util/CMakeLists.txt

@@ -7,20 +7,36 @@ sourcelist_to_parent(share_genf90_sources)
 
 list(APPEND share_sources "${share_genf90_sources}")
 
-list(APPEND share_sources shr_file_mod.F90 shr_kind_mod.F90 shr_const_mod.F90
-  shr_sys_mod.F90 shr_log_mod.F90 shr_orb_mod.F90 shr_spfn_mod.F90 shr_strconvert_mod.F90
-  shr_nl_mod.F90 shr_precip_mod.F90 shr_string_mod.F90 shr_timer_mod.F90 shr_vmath_mod.F90
+list(APPEND share_sources
+  shr_file_mod.F90
+  shr_kind_mod.F90
+  shr_const_mod.F90
+  shr_sys_mod.F90
+  shr_log_mod.F90
+  shr_orb_mod.F90
+  shr_spfn_mod.F90
+  shr_strconvert_mod.F90
+  shr_nl_mod.F90
+  shr_precip_mod.F90
+  shr_string_mod.F90
+  shr_timer_mod.F90
+  shr_vmath_mod.F90
   shr_wv_sat_mod.F90)
 
 # Build a separate list containing the mct wrapper and its dependencies. That
 # way, this list can be easily included in unit test builds that link to mct,
 # but excluded from builds that do not include mct.
-list(APPEND share_mct_sources mct_mod.F90 shr_mct_mod.F90 shr_mpi_mod.F90 shr_pcdf_mod.F90)
+list(APPEND share_mct_sources
+  mct_mod.F90
+  shr_mct_mod.F90
+  shr_mpi_mod.F90
+  shr_pcdf_mod.F90)
 
 # Build a separate list containing the pio wrapper and its dependencies. That
 # way, this list can be easily included in unit test builds that include PIO or
 # a stub of PIO, but excluded from builds that do not include PIO.
-list(APPEND share_pio_sources shr_pio_mod.F90)
+list(APPEND share_pio_sources
+  shr_pio_mod.F90)
 
 sourcelist_to_parent(share_sources)
 sourcelist_to_parent(share_mct_sources)