signac-flow icon indicating copy to clipboard operation
signac-flow copied to clipboard

Published 20 hours ago • verified publisher icon glotzerlab

Refactor testing code

Open bdice opened this issue 3 years ago • 4 comments

Motivation

While working on PR #608, I had some difficulty writing tests for traceback handlers. That sparked a discussion about how we could improve our testing code in signac-flow. This is a meta-issue meant for discussion of potential improvements to the testing code.

Potential Improvements

(Please suggest other improvements in comments on this issue. We can identify which are most actionable and create separate issues / PRs to address.)

  • Both status tests and template tests have a complicated set of scripts to create outputs for the current code and compare them to data stored in a tarball (for identifying regressions). Maybe we can unify the code between status tests and template tests, and minimize duplication.
  • Testing the signac-flow CLI (python project.py run) currently requires a file on disk like define_test_project.py that can be passed as an entrypoint. This involves a lot of mocking code to create an environment that acts as if it a user FlowProject acting on user data. I wonder if we can do something that is more self-contained or reduces the amount of mocking needed. Perhaps even consolidating define_test_project.py and similar modules into a subfolder would help.

bdice avatar Feb 16 '22 22:02 bdice

Here are some thoughts on how we could approach this:

  1. Use a highly modular fixture hierarchy for everything. The fixture concept is the most powerful pytest feature IMO and we should take full advantage of that. I would identify the tests with complicated "data" setups and then start replacing them one by one.
  2. Fixtures should be modular. A project fixture should be built on top of job fixtures, which should be built on docs and statepoint fixtures which are built with individual data fixtures and so on.
  3. Fixtures that are used across all test modules should be defined in conftest.py. Fixtures that are autoused and never explicitly used in the tests functions should be private.
  4. Essentially all tests that are operating on data structures (individual documents, jobs, projects) should use standard (parameterized) fixtures. That has the huge advantage that any test that is operating on some kind of job, can automatically be be tested against all kinds of jobs with varying data structures. Filter queries can be fixtures, too!
  5. Isolate the test environment. A fixture with monkeypatch and autouse can be employed to automatically create an isolated test environment.
  6. Use a cli-runner fixture for CLI tests. The click project provides the CLIRunner which makes it very easy to write CLI tests. Unfortunately the previous attempt to refactor our CLI with click failed, but I would hope that there is a similar fixture provided by a pytest plugin that provides an equivalent functionality (if not we should write it).

One more thing: The branch on which we would perform this refactoring cannot change any of the module code to guarantee that we do not inadvertently change the behavior.

csadorf avatar Feb 17 '22 12:02 csadorf

I should split this up into smaller parts so that the code becomes easy to review. I'll probably raise smaller, more focused issues for refactoring this code.

kidrahahjo avatar Mar 08 '22 07:03 kidrahahjo

I created a branch test-refactor/main to resolve all the testing related issues. The logic of this branch is similar to that of the next branch that we usually create.

kidrahahjo avatar Jun 01 '22 07:06 kidrahahjo

Tests/Classes which need to be rewritten/refactored for signac-flow testing:

tests/test_project.py

  • [x] Mock Scheduler
  • [x] Mock Environment
  • [ ] MockSchedulerSubmitError
  • [x] TestProjectBase
  • [ ] TestProjectStatusPerformance
  • [ ] TestProjectStatusNoEligibleOperations
  • [ ] TestProjectClass
  • [ ] TestProject
  • [ ] TestExecutionProject
  • [ ] TestExecutionDynamicProject
  • [ ] TestProjectMainInterface
  • [ ] TestDynamicProjectMainInterface
  • [ ] TestDirectivesProjectMainInterface
  • [ ] TestProjectDagDetection
  • [ ] TestProjectSubmitOptions
  • [ ] TestGroupProject
  • [ ] TestGroupExecutionProject
  • [ ] TestGroupExecutionDynamicProject
  • [ ] TestGroupProjectMainInterface
  • [ ] TestGroupDynamicProjectMainInterface
  • [ ] TestAggregatesProjectBase
  • [ ] TestAggregatesProjectUtilities
  • [ ] TestAggregationProjectMainInterface
  • [ ] TestAggregationGroupProjectMainInterface
  • [ ] TestHooksSetUp
  • [ ] TestHooksBase
  • [ ] TestHooksCmd
  • [ ] TestHooksInstallSetUp
  • [ ] TestHooksInstallBase
  • [ ] TestHooksInstallCmd
  • [ ] TestHooksInstallWithDecorators
  • [ ] TestHooksInstallCmdWithDecorators
  • [ ] TestHooksInstallNoDecorators
  • [ ] TestHooksInvalidOption
  • [ ] TestIgnoreConditions

tests/test_aggregates.py

  • [x] AggregateProjectSetup
  • [x] TestAggregate
  • [x] TestAggregateStore

tests/test_directives.py

  • [ ] TestItems
  • [ ] TestDirectives

tests/test_environment.py

  • [ ] TestProject

tests/test_shell.py

  • [ ] TestCLI

tests/test_status.py

  • [ ] test_print_status

tests/test_templates.py

  • [ ] test_env

tests/test_util.py

  • [ ] TestBidict
  • [ ] TestTemplateFilters

kidrahahjo avatar Jun 01 '22 09:06 kidrahahjo