Skip to content

Jason's Coding Exercise#1

Open
parappayo wants to merge 15 commits intomainfrom
jestey-solution
Open

Jason's Coding Exercise#1
parappayo wants to merge 15 commits intomainfrom
jestey-solution

Conversation

@parappayo
Copy link
Collaborator

This is more than I'd normally put in a single PR. It would probably be easier to review (and course correct early) if I just implemented one or two endpoints per PR, including tests.

In brief, this PR makes the following changes:

  • Implemented endpoints defined in handler.go (addresses TODOs there)
  • Implemented data access funcs in store.go (addresses TODOs there)
  • Used Copilot to generate tests for DocumentStore and create a mock to represent the db layer
  • Ran go mod tidy (most of the changes to go.mod)

Testing at the DocumentStore layer is probably the most effective for these changes. By using a mock we don't need to run Postgres. Normally I'd put more work into trying to define some smaller, more "business logic" centric tests. The tests that Copilot created are at least useful for basic code coverage.

It would also be good to define automated tests against the web API itself, by running the server and issuing POSTs and GETs with validation against the responses. I did manual verification using curl and Insomnia (like Postman.) Normally something like that would run in the CI pipeline to check for errors before deploying and/or run tests after deploying to a QA environment. No such tests are defined here.

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