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

Refactor testing code #611

Open
bdice opened this issue Feb 16, 2022 · 4 comments
Open

Refactor testing code #611

bdice opened this issue Feb 16, 2022 · 4 comments

Comments

@bdice
Copy link
Member

bdice commented Feb 16, 2022

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.
@csadorf
Copy link
Contributor

csadorf commented Feb 17, 2022

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.

@kidrahahjo
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

kidrahahjo commented Jun 1, 2022

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

tests/test_project.py

  • Mock Scheduler
  • Mock Environment
  • MockSchedulerSubmitError
  • 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

  • AggregateProjectSetup
  • TestAggregate
  • 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

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

No branches or pull requests

3 participants