Skip to content

docs(consumer): create list of best practice rules #383

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mefellows
Copy link
Member

@mefellows mefellows commented May 12, 2025

I took a shot at extracting the best practices in the video, and transposing them into a useful set of rules people could use to review tests.

I'm not 100% happy with them, but thought I'd share my thinking (the "Bug Catcher" rule I'm struggling to define clearly, but probably because it's quite a difficult thing to generalise but also give concrete examples concisely).

@mefellows mefellows requested a review from YOU54F May 12, 2025 11:31

**❌ Bad:**
```ts
.uponReceiving("a request for a list of products") // no provider state
Copy link
Member

Choose a reason for hiding this comment

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

maybe expand this to contain two tests with same description, one expecting a 200 / 404 or empty list

@YOU54F
Copy link
Member

YOU54F commented May 12, 2025

This looks great.

for the bug catcher rule - we could show a decision engine response, and dependant on the status returned, the client does and returns something different.

removing one of the tests would lean to an enum case and associated code tree not being tested

@mefellows mefellows requested a review from rholshausen May 12, 2025 22:56
@rholshausen
Copy link

All makes sense to me

Copy link

@vwong vwong left a comment

Choose a reason for hiding this comment

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

LGTM.

Nice that you added Authorization: Bearer xxx since OPC now checks for that explicitly.

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.

4 participants