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

Make PyCIEMSS testable #32

Merged
merged 12 commits into from
Aug 10, 2023
Merged

Make PyCIEMSS testable #32

merged 12 commits into from
Aug 10, 2023

Conversation

fivegrant
Copy link
Collaborator

@fivegrant fivegrant commented Aug 8, 2023

At a high level this PR

  1. Reduces the amount code in the API and the worker and moves most into the model itself
  2. Tests the conversions of models into keyword arguments for the PyCIEMSS library
  3. Formats the code using Black and Ruff
  4. Defines GitHub Actions for formatting and testing

To see how to take advantage of these new features, checkout the Testing section in the README.

The PyCIEMSS Service is only a light wrapper that enables REST interaction with the PyCIEMSS library. Moving most of the logic out of the worker and into the model isolates the components that are subject to change when either the contract or the underlying PyCIEMSS library change.

@fivegrant fivegrant requested a review from brandomr August 8, 2023 15:15
@fivegrant fivegrant changed the base branch from main to dev/1.6.0 August 8, 2023 15:15
@brandomr
Copy link

brandomr commented Aug 8, 2023

i'd rename operators.py to tests.py for clarity and also please add a README to the tests directory or somewhere else you'd like that explains the tests, how to run them and how to add one.

@fivegrant
Copy link
Collaborator Author

@brandomr I made your suggested change for now. I'm anticipating that more tests outside of the operation models may arise in the future and it may make sense to have two separate files. There's really no reason to future proof for this though so I'm happy making the change

@fivegrant
Copy link
Collaborator Author

fivegrant commented Aug 8, 2023

Outstanding TODOs:

  • Fix CI
  • Test function signature against PyCIEMSS

@fivegrant fivegrant merged commit 63993f1 into dev/1.6.0 Aug 10, 2023
2 checks passed
fivegrant added a commit that referenced this pull request Aug 10, 2023
* Refactor glue code into models

* Remove operations

* Install dev tools

* Add tests [mocking broken]

* Test if visual is True

* Document how to test

* Add CI for tests and formatting

* Link to file instead of URL for dataset

* Remove yaml lint

* Simplify API

* Fix imports on server

* Fix tests path
@fivegrant fivegrant deleted the fg/model-consolidate branch August 10, 2023 14:06
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