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: misc improvements in tests #60

Merged
merged 21 commits into from
Nov 20, 2023
Merged

refactor: misc improvements in tests #60

merged 21 commits into from
Nov 20, 2023

Conversation

ComradeVanti
Copy link
Collaborator

This PR was originally meant to introduce property-based tests (see #59). After some experiments it did not seem to add to much test security, so this was abandoned (Notice fast-check dependency was added and later removed)

Instead this PR includes various small changes that can be summarized with:

  • Fixed a few small type errors in the actual implementation code, based on type usage in tests (For example make some properties optional)
  • Add typings in tests where applicable
  • Introduce/refactor helper functions and types for
    • Mock package registry
    • Mock console
    • Mock manifest
    • Mock work directory
  • Deduplicate a lot of code by adding helper
  • Split utilities.ts and group common functionality in new modules
  • Small dev-dependency version changes to fix warnings

Add type information indicating that result code will always be either 0 or 1
Use information found in test usage to adjust types throughout the project. Also type constants in tests
Add utility functions to de-duplicate the common logic of adding packages and search results to the mock package registry in tests
Split utilities file into mock-work-dir and mock-console
Hide implementation detail of what mock console is used from tests
Add helpers  to check for correct manifest in add cmd tests
Move logic for, creating, resetting and querying mock console content into dedicated functions
To avoid warnings. Only dev dependencies
Move manifest assertion helpers to own module so they can be reused
prepare for allowing to chain multiple assertions on same manifest
Use manifest assertions in cmd remove tests. Also add a few new ones
Use manifest assertions in manifest tests
There should never be a version inside the name
Copy link
Member

@favoyang favoyang left a comment

Choose a reason for hiding this comment

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

GLTM. The refactoring has made the unit tests look much cleaner.

@favoyang favoyang merged commit 3abf506 into openupm:master Nov 20, 2023
2 checks passed
@ComradeVanti ComradeVanti deleted the test-refactor branch November 20, 2023 14:38
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.

2 participants