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

crates/service test coverage #358

Closed
mxfactorial opened this issue Apr 23, 2024 · 3 comments · Fixed by #359
Closed

crates/service test coverage #358

mxfactorial opened this issue Apr 23, 2024 · 3 comments · Fixed by #359
Assignees

Comments

@mxfactorial
Copy link
Contributor

current

absent crates/service unit test coverage

expected

crates/service unit test coverage

the Service struct stores a generic conn value. but supporting mock assertions requires changing conn to a reference. and if conn is a reference, then it must also be mutable since the create_transaction_tx function requires a mutable reference to self. now the database connection is stored as a reference to a mutable, generic value just to enable testing. may as well be javascript

short term solution:

  1. separate the create_transaction_tx function into its own service
  2. change conn to a reference
  3. add test coverage to the reduced Service impl

long term solution:

  1. create a composite postgres type that combines transaction, transaction_item and approval
  2. replace the tokio_postgres transaction requiring a mutable reference to self with a postgres function. switching to a postgres function also avoids creating a massive common table expression to insert on the transaction, transaction_item and approval tables

hourly estimate

  1. 32 service impl functions x 10 mins: 5
  2. make: 0.25
  3. workflows: 1
  4. testing: 2
  5. doc: 0

8.25

@mxfactorial mxfactorial self-assigned this Apr 23, 2024
@mxfactorial
Copy link
Contributor Author

hours worked

requirement writing: 2.5

@mxfactorial
Copy link
Contributor Author

mxfactorial commented Apr 24, 2024

hours worked

6.25

@mxfactorial
Copy link
Contributor Author

long term solution delivered in #360

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

1 participant