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

CMake Tests 3 and 8 fail due to new test case, example 6, added in e636f96 #30

Closed
zbeekman opened this issue Dec 30, 2014 · 11 comments · Fixed by #64
Closed

CMake Tests 3 and 8 fail due to new test case, example 6, added in e636f96 #30

zbeekman opened this issue Dec 30, 2014 · 11 comments · Fixed by #64

Comments

@zbeekman
Copy link
Contributor

The CMake tests are crude, and intentionally introducing an invalid json file and diagnosing the error is causing CTest to hit on the ‘error’ regex and incorrectly say that the test has failed.

My preferred fix would be to break up the json_example.f90 file into individual tests, one per file, then they could be tested one by one. What do you think @jacobwilliams ?

@jacobwilliams
Copy link
Owner

I think it's a good idea. The unit test need some cleanup (see Issue #27). Testing all the error cases is especially important (where the error happening and being caught is the success).

zbeekman referenced this issue Dec 30, 2014
…is parsed. Added a new test case with an invalid file.
@zbeekman
Copy link
Contributor Author

Do you have any preferences for directory structure and naming convention if we split json_example.f90 into individual tests? I’m gonna tackle this now I think.

@jacobwilliams
Copy link
Owner

Maybe a tests directory? With file names test1.f90, test2.f90, etc.

@zbeekman
Copy link
Contributor Author

Under src, or along side?

@jacobwilliams
Copy link
Owner

Hmmm.. not sure. Maybe under src?

@zbeekman
Copy link
Contributor Author

Right now this is what I’m thinking in terms of splitting out the tests:

  • Move json_example.f90 to json_unit_tests.f90 and make it a module rather than a program.
  • Retain the executable program portion of json_example.f90 and USE the json_unit_tests module to pull in the test procedures. This means that very little modification of the build systems is required, and the example program is still fully functional, but split across two files.
  • This is where we have some choices
    • I’m tempted to use CMake’s ability to automatically generate test drivers which would:
      1. require a C compiler be installed to run the CMake unit tests—this is almost guaranteed if a Fortran compiler is installed, but I have mixed feelings about this
      2. automatically generate the test driver program, and compile and link in the fortran tests (with iso_c_binding wrappers)
      3. make writing the test effortless
      4. allow us to create things like a CDash dashboard
      5. mean that the Scons and build.sh build/test scripts would still just run unified tests in json_example.f90 but CMake and CTest would have more fine grained test control and results. The fine grained CMake/CTest test control can be implemented either way, but will be more work without using the automatic test driver build.
    • OR we could just create a number of stand alone programs, which, for now, call the procedures in the json_unit_tests module.
      1. We’ll have to script each of them to be called in each build system
      2. We’ll need some way in each build system to check for success or failure
        • Could use regexp matching
        • Comparison with known valid output?

Any feedback appreciated esp. CC: @jacobwilliams

@jacobwilliams
Copy link
Owner

I would prefer the simpler option of just having stand-alone programs, with each one a separate test. And then the master unit test script (Python maybe) calls each of them and determines if each one passes or fails. I know there are unit-testing frameworks out there, I'm just not familiar with the different options.
I would choose whatever option is the simplest, easy to use and add new tests, and easiest for a poor Fortran man to understand!

@zbeekman
Copy link
Contributor Author

Well… the cmake one just requires you to write a bind(c) function for each test, put it in its own file having the same name as the function, and add it to a list of test sources in CMake… CMake builds and links etc for you and you can run all test with make test or ctest --verbose so it is actually quite simple for the end user… they just need a c compiler, but they don’t need to know how to use it.

btw do you have any insight on updating the SConstruct build in the case that json_example.f90 has been spit into json_unit_tests.f90 (module) and json_example.f90 (main program)

@zbeekman zbeekman mentioned this issue Feb 18, 2015
@zbeekman
Copy link
Contributor Author

OK, I have thought more about this, and I have come up with what I think is a much better and simpler solution than the one I proposed above:

  1. Move build.sh —> build-and-test.sh, building and running tests unless environment variable $JF_SKIP_TESTS is set to true
  2. Completely refactor json_example.f90 into individual test programs and get rid of json_example.f90 altogether—people looking for usage examples can just look at the unit tests directly.
  3. Each test subroutine from json_example.f90 will now be its own program stored in a file named jf_test_*.f90. This will let us give tests descriptive or numeric names, while allowing us to vacuum up all test programs in the tests directory with some simple file globs. This will also make it easy to run the tests with CMake.
  4. Each of these test programs will use stop 1 statements to let the driver script know an error occurred. Both GFortran and Intel compilers will set $?, the return value, of programs according to the stop statement. With set -e in build-and-test.sh this means that the travis-ci will stop and report failures as soon as the first test encounters an error. There are ways to change this so that it will attempt all the tests, which could be desirable (although will take longer) and we can change this if that’s what you prefer.
  5. All test programs print informational messages to stderr and any json output to stdout, so that it may be redirected into jsonlint or similar for validation.
  6. If, in the future, we want to call the tests from some test harness/driver we can always have each source file contain a module with the test subroutine and then use conditional compilation with preprocessing to determine whether the stand alone program should be compiled (to avoid multiple definitions of _main):
module js_test_strprint_mod
use json_module
implicit none
! constants etc here
contains
subroutine test_strprint(err_count)
  integer, intent(out) :: err_count
  ! do tests
end subroutine
end module

#ifndef NO_STAND_ALONE_TESTS
 program jf_test_strprint
   use jf_test_strprint_mod
   implicit none
   integer :: errors
   call  test_strprint(errors_
   if (errors /= 0) stop 1
 end program
#endif

@jacobwilliams do you think this is simple/KISS enough for “fortran poor man”? 😄

@jacobwilliams
Copy link
Owner

That sounds great!! I especially like the idea of being able to add a new jf_test_* file to the tests directory, and it is automatically included in the test run, rather than having to update all the various build files.
Also, it might be good for the build script to have a command line flag like -skiptests if you don't want to run the tests. A poor fortran man doesn't know how to set environment variables. Ha ha!

@zbeekman
Copy link
Contributor Author

HAHA, well this “poor fortran man” is a bit rusty on bash command line flags (I know you use getopt or something like that…) so I may end up leaving that for someone else to implement… we’ll see.

@zbeekman zbeekman mentioned this issue Feb 27, 2015
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 27, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -Breaks SCons build: @jacobwilliams and @bruceravel, my python/SCons is too nonexistant to fix this myself.
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 27, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -Breaks SCons build: @jacobwilliams and @bruceravel, my python/SCons is too nonexistant to fix this myself.
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 27, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -Breaks SCons build: @jacobwilliams and @bruceravel, my python/SCons is too nonexistant to fix this myself.
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 27, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -Breaks SCons build: @jacobwilliams and @bruceravel, my python/SCons is too nonexistant to fix this myself.
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, SCons and VS builds likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 28, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -SCons build works and runs test with `scons test`
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, VS build likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
 -Small CMake bug fixed where CMake wasn't passing the requested GNU specific flags
 -CMake and build.sh have added options to compile for code coverage analysis with gcov when using GNU compiler
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 28, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -SCons build works and runs test with `scons test`
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, VS build likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
 -Small CMake bug fixed where CMake wasn't passing the requested GNU specific flags
 -CMake and build.sh have added options to compile for code coverage analysis with gcov when using GNU compiler
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 28, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -SCons build works and runs test with `scons test`
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, VS build likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
 -Small CMake bug fixed where CMake wasn't passing the requested GNU specific flags
 -CMake and build.sh have added options to compile for code coverage analysis with gcov when using GNU compiler
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 28, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -SCons build works and runs test with `scons test`
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, VS build likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
 -Small CMake bug fixed where CMake wasn't passing the requested GNU specific flags
 -CMake and build.sh have added options to compile for code coverage analysis with gcov when using GNU compiler
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 28, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -SCons build works and runs test with `scons test`
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, VS build likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
 -Small CMake bug fixed where CMake wasn't passing the requested GNU specific flags
 -CMake and build.sh have added options to compile for code coverage analysis with gcov when using GNU compiler
zbeekman added a commit to zbeekman/json-fortran that referenced this issue Feb 28, 2015
 -Fixes jacobwilliams#30: CMake tests 3 and 8 fail
 -Fixes jacobwilliams#49: tests dir should be under src
 -Maybe addresses jacobwilliams#27... I'll leave that up to @jacobwilliams
 -maybe concludes jacobwilliams#42...
 -SCons build works and runs test with `scons test`
 -Probably breaks visual studio build, @jacobwilliams: I can't work on this, I don't have access to visual studio
 -Tests return the number of errors encountered
 -CMake and build.sh scripts work fine, VS build likely broken
 -CMake flags now distinguished between compiler flags (for all compilers) and fortran specific compiler flags
 -Small CMake bug fixed where CMake wasn't passing the requested GNU specific flags
 -CMake and build.sh have added options to compile for code coverage analysis with gcov when using GNU compiler
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 a pull request may close this issue.

2 participants