Adds DevMock Service #7

Krakabek · 2024-08-21T13:44:04Z

Description:

Dev Mock Service Worker is an API mocking library that allows you to specify custom responses for any API calls inside your app.

🧪 Testing Steps / Validation

Unit tests
NPM link on a real app to verify the mocked request works

✅ Checks

Contributor License Agreement (CLA) completed if not a Reddit employee

niedzielski

looks like it could be very useful! only concerns are: bundle size and confusion over all the "devv" and "dev" terminology which seems like it's just mocking.

src/dev-mock-service/redis-mock-service/types.ts

src/dev-mock-service/redis-mock-service/index.ts

niedzielski · 2024-08-21T14:47:02Z

src/dev-mock-service/redis-mock-service/index.ts

+} from "./types.js";
+import type { HandlerOverride } from "../types/internal.js";
+
+export const createDevvRedis = (


suggest calling this createDevvitRedis(). I don't think we use Devv anywhere in any of the codebases (rg -w Devv comes up empty in shreddit and devvit repos). would love to not have a new term to grep. that said, I think everything here is pretty devvit-y so also consider newMockRedis(). same request for createDevvRedditApi(), DevvRedditApi, and everywhere else.

I am not sure about the naming here, because it's not a mockRedis, it's an instance that intercepts some redis requests, but fires real ones for the rest
So calling it mockRedis might lead to an expectation that it only performs mock requests

Devv was a way to do the reference to devvit and developer at the same time, and I did not come up with a better option yet

src/dev-mock-service/redis-mock-service/index.test.ts

src/dev-mock-service/reddit-api-mock-service/index.ts

niedzielski · 2024-08-21T15:15:24Z

src/dev-mock-service/README.md

@@ -0,0 +1,47 @@
+## Dev Mock Service


🔕 suggest consider renaming to just "mocks". "dev" and "service" aren't buying me much. same thing with the subdirectory names which could be like mock-reddit instead of reddit-api-mock-service.

niedzielski · 2024-08-21T15:19:11Z

src/dev-mock-service/README.md

+Redis, RedditAPI, or HTTP request.
+
+- In Dev mode (`DevMockMode.Dev`), handlers are applied for all requests with the matching method and ID (if available).
+- In Prod mode (`DevMockMode.Prod`), handlers are ignored.


this will encourage users to ship development-only code which may be large. I'd love to see this code fall out of apps for at least non-prerelease builds so there wasn't a wrong way to use it.

How can I achieve this?

I'd like the introduction of a IS_PROD or NODE_ENV or IS_PLAYTEST environmental variable that could be statically added at build time for tree shaking. Would be useful for more than just here

I wonder if we can implement the IS_PLAYTEST and tree shaking separately, so it does not block this PR

niedzielski · 2024-08-21T15:26:19Z

src/dev-mock-service/dev-mock-service.ts

+  devvRedis: RedisClient;
+  devvRedditApi: RedditAPIClient;
+  devvFetch: typeof fetch;


suggest omitting the name devv prefix names (I make a comment about this later that it's a new term) because I don't think it's adding much useful context here. the user only needs to know which service is which and the typing covers that.

This is done to avoid confusion with the actual instances of redis and reddit
Imagine the following scenario

const {redis, reddit} = context; const { devvRedis, devvRedditApi, devvFetch } = DevMock.createService({...

Without the prefix there is a potential name collision

src/dev-mock-service/http-mock-service/index.ts

niedzielski · 2024-08-21T15:30:16Z

src/dev-mock-service/http-mock-service/types.ts

@@ -0,0 +1,6 @@
+export type HttpOverride = {


I think users end up interacting with this type a bit? if so, consider documenting in JSDocs.

This is a purely internal type, used to filter overrides based on the type

mwood23

I really appreciate you taking the time to mock all of these services out! I think I need an example to understand it more. It looks like a design goal here was to avoid any alterations to public-api or the underlying build system which I understand. I do wish this is something we could do without touching the app code.

Some areas I'd consider exploring:

Everything is mocked by default and a helpful error is throw if a network request is triggered. I think this is a sound default since an app only has one redis. Running the tests and triggering a bunch of writes to the DB would be bad vibes.
Is there a way to alter the implementation details of these functions under the hood to make it transparent to the caller. For example, when running the tests, what if we found a way to mutate the context singleton automatically instead of putting a function in front of them.

This triggers some folks (and I totally understand), but I'm a fan of writing my own test harness that wrap describe and it. Then you have take care of isolating stuff in true integration tests (aka creating a new userId per test so you can parallelize them). Or, in this case, automatically restore all of the mocks. Something like this:

import { DevvitTestHarness } from 'somewhere'

DevvitTestHarness.it('should do xyz', [
DevMock.fetch.get("https://example.com", () =>
      DevMock.httpResponse.ok({ fetched: "mock" }),
    )
], () => {
expect('https://example.com').toBe('mock')
})

mwood23 · 2024-08-26T14:07:07Z

src/dev-mock-service/README.md

@@ -0,0 +1,47 @@
+## Dev Mock Service
+
+Dev Mock Service Worker is an API mocking library that allows you to specify


This isn't actually a server worker is it?

Whoops, bad copy paste 😅

mwood23 · 2024-08-26T14:08:21Z

src/dev-mock-service/README.md

+Redis, RedditAPI, or HTTP request.
+
+- In Dev mode (`DevMockMode.Dev`), handlers are applied for all requests with the matching method and ID (if available).
+- In Prod mode (`DevMockMode.Prod`), handlers are ignored.


I'd like the introduction of a IS_PROD or NODE_ENV or IS_PLAYTEST environmental variable that could be statically added at build time for tree shaking. Would be useful for more than just here

mwood23 · 2024-08-26T14:11:20Z

src/dev-mock-service/reddit-api-mock-service/index.ts

+import type { HandlerOverride } from "../types/index.js";
+import type { ModMailService } from "@devvit/public-api";
+
+class DevvRedditApi implements RedditApiInterface {


Thanks for all of the leg work here!

mwood23 · 2024-08-26T14:13:44Z

src/dev-mock-service/README.md

+
+```typescript
+const redisValue = await devvRedis.get("mocked_key"); // "Value from mocks!"
+const fetchedValue = await(await devvFetch("https://example.com")).json(); // {fetched: "mock"}


nit: double await sort of always been a weird pattern to me instead of an intermediate var. Especially to noobies

This is just an example to demonstrate the responses
Do you think it's worth breaking into 3 sections? redis, reddit and fetch separately

mwood23 · 2024-08-26T14:15:00Z

src/dev-mock-service/README.md

+
+#### Setup
+
+Create devv versions of the API clients you want to mock.


Is this in the test file or in the real app code?

Real app code, this molecule is not intended to be used in unit tests (however it certainly can be)

danil.radkovskyi added 2 commits August 21, 2024 15:29

Initial commit for dev mock service

d9c674f

Update docs

6678478

Krakabek requested review from niedzielski, mwood23, ben-harris-5 and stacy-qqq August 21, 2024 13:44

Krakabek self-assigned this Aug 21, 2024

niedzielski approved these changes Aug 21, 2024

View reviewed changes

danil.radkovskyi added 4 commits August 22, 2024 13:49

PR fixes

c760a1b

run formatter

b60b743

fix build

3033cd0

Fix naming

fb8dc30

mwood23 approved these changes Aug 26, 2024

View reviewed changes

danil.radkovskyi added 2 commits August 27, 2024 14:27

Fix copy paste mistake

fd9b7f0

Rename devv to dev

2af0a5f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds DevMock Service #7

Adds DevMock Service #7

Krakabek commented Aug 21, 2024

niedzielski left a comment

niedzielski Aug 21, 2024

Krakabek Aug 22, 2024

niedzielski Aug 21, 2024

niedzielski Aug 21, 2024

Krakabek Aug 22, 2024

mwood23 Aug 26, 2024

Krakabek Aug 27, 2024

niedzielski Aug 21, 2024

Krakabek Aug 22, 2024

niedzielski Aug 21, 2024

Krakabek Aug 22, 2024

mwood23 left a comment

mwood23 Aug 26, 2024

Krakabek Aug 27, 2024

mwood23 Aug 26, 2024

mwood23 Aug 26, 2024

mwood23 Aug 26, 2024

Krakabek Aug 27, 2024

mwood23 Aug 26, 2024

Krakabek Aug 27, 2024

		@@ -0,0 +1,47 @@
		## Dev Mock Service

		Dev Mock Service Worker is an API mocking library that allows you to specify


		#### Setup

		Create devv versions of the API clients you want to mock.

Adds DevMock Service #7

Are you sure you want to change the base?

Adds DevMock Service #7

Conversation

Krakabek commented Aug 21, 2024

Description:

🧪 Testing Steps / Validation

✅ Checks

niedzielski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwood23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment