Skip to content

Allow selective testing of single suites and tests #177

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

Merged
merged 2 commits into from
Sep 20, 2020

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Sep 15, 2020

Related to #176.

In case we want to continue using the current unit testing framework, this PR should reduce the boilerplate code to register new test suites and allow for some better debugging of failing tests.

Registering a new test suite works now in a similar way as registering a unit test in the collecting interface:

fpm/fpm/test/main.f90

Lines 15 to 19 in 4a5ecae

testsuite = [ &
& new_testsuite("fpm_toml", collect_toml), &
& new_testsuite("fpm_manifest", collect_manifest), &
& new_testsuite("fpm_source_parsing", collect_source_parsing) &
& ]

The other point is handling and debugging of failing tests.

Running fpm test will run all test suites and all contained unit tests. In case we encounter a failure, we usually don't want to run all tests every time while debugging, therefore I added the option to select the test suite and the test we want to run.

The available levels of testing are than:

  1. fpm test: no argument will run all available test suites
  2. fpm test --args "help" or any other not available test suite will print the names of all available test suites (will not run any tests and exit code will be 1)
# Available testsuites
# - fpm_toml
# - fpm_manifest
# - fpm_source_parsing
  1. fpm test --args "fpm_source_parsing" will select the source parsing test suite and only run its unit tests
  2. fpm test --args "fpm_source_parsing help" or any other not available unit test will print the names of all available unit tests in the test suite source parsing (will not run any tests and exit code will be 1)
# Suite: fpm_source_parsing
# Available tests:
# - modules-used
# - intrinsic-modules-used
# - include-stmt
# - module
# - submodule
# - submodule-ancestor
# - subprogram
# - csource
# - invalid-use-stmt
# - invalid-include-stmt
# - invalid-module
# - invalid-submodule
  1. fpm test --args "fpm_source_parsing invalid-submodule" will only run the invalid-submodule unit test from the source parsing test suite

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

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

This is a very useful addition, thanks! I see no problems with the implementation. 👍

@awvwgk awvwgk mentioned this pull request Sep 16, 2020
@milancurcic
Copy link
Member

Thanks, @awvwgk. I have a few questions.

  1. Does the argument value really need the quotes around it?
fpm test --args "help"

or does this work:

fpm test --args help

If not, any way we can make it work without quotes? As is, the CLI UI seems a bit awkward to me.

  1. Perhaps the quotes are needed to pass multiple values to --args? I wonder if we need args at all. What do you think about this API instead:
fpm test                                # runs all test suites
fpm test fpm_source_parsing             # runs only this test suite
fpm test missing_test_suite             # shows available test suites if missing test suite is requested
fpm test test_suite_one, test_suite_two # list multiple suites separated by comma
fpm test --help                         # shows the help message and the list of available test suites
fpm test -h                             # same as above

This way we drop the --args and quotes altogether, IMO for a cleaner UI.

@awvwgk
Copy link
Member Author

awvwgk commented Sep 18, 2020

Regarding the first point, for a single argument it will work without quotes just fine. The reason for the quotation marks is mainly due to the limitation of the bootstrap fpm implementation, see #138. Therefore, I just put them around the arguments every time while testing (and writing the PR up).

Regarding your suggestion at the second point, I guess there is some mix-up between the testing framework in fpm and the fpm-test target. Since we are building and testing fpm with fpm, this requires careful reading and writing, I hope I got the latter right.

  1. fpm test would invoke the test targets for fpm, which contain all executables contained in [[test]] sections
  2. fpm test fpm-test would invoke the fpm-test target defined for fpm (for fpm this is identical to 1.)
  3. fpm test test_source_parsing would be ambiguous, there is no target called test_source_parsing, in case there is only one test target available, we could allow fpm to be smart and pass it as argument to the one test target in fpm. I think this is rather dangerous, in case the test executable doesn't handle command line arguments it will fail silently.
  4. fpm test missing_test_suite is in this regard identical to 3.
  5. fpm test --help should print the help on the fpm test command, not pass this as argument to the test target.

The best solution would be to allow the -- flag in bootstrap fpm to pass every following argument to the selected target, currently we have to use --args "<arg1> ..." due to #138.

@milancurcic
Copy link
Member

Okay, if I understood this correctly, I confused fpm's internal test suites with test targets that any fpm package can have. In that case, I'm happy with this moving forward as is.

@awvwgk awvwgk mentioned this pull request Sep 18, 2020
@milancurcic
Copy link
Member

This is a small and low impact PR with no objections so I'll go ahead and merge it. Thank you, @awvwgk!

@milancurcic milancurcic merged commit e79b47e into fortran-lang:master Sep 20, 2020
@awvwgk awvwgk deleted the selective-testing branch September 20, 2020 16:08
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.

3 participants