-
Notifications
You must be signed in to change notification settings - Fork 8
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
RFC Support async customHandler #3
Comments
Hi, I think that sounds great! It'd be nice to integrate this with as little intervention as possible, as you're also on to in your comment. Building on your thought about graphql-query-test-mock/src/getNockRequestHandlerFn.js Lines 92 to 96 in 6ba42a2
That'd work I think, provided that the mock can resolve synchronously. What do you think about that? A PR would be great, sure! |
Unfortunately that would not work because the function needs to resolve asynchronously. This is because I'm going to be calling an embedded
From a code organization perspective, I think adding async support to You can count on me to do my best to refactor Btw, would you prefer the async function to be a function that takes a node-style error-first callback, or a function that returns a promise? |
Hey again, Right! I do agree with you about making What do you think about that approach? Please feel free to play around with that. Like I said, we'll make I guess we'll want to add a way of adding custom mock resolvers as well when wanted, which I saw |
Sure. I completely agree automocking would be a great feature. I'm not sure yet of what the right API would look like for it since I didn't get a chance to experiment with it yet. Here are my current assumptions of what I think I need in order to start using this feature:
Some other considerations / open questions I have:
I would love to hear your thoughts on that before we go more into the specifics of the implementation. Re
My What do you think? |
Hi again! Sorry for not getting back to you sooner, holidays ;) I think making
I look forward to seeing what you come up with in your experiments! Then we can spend some more time thinking about how to scale this approach, how it'd make the most sense, and if it needs to be implemented in another way. |
That's great! I've already started using your changes and it's been helpful to have access to the query text and the variables. Btw it looks very promising. I was able to shrink a test file by 70% without losing any confidence in my tests. The mergeResolver script works ok for now but it could be better. I will ping this thread by the end of January to share my findings. Happy new year! |
Hey @jnak and a happy new year even though I missed it by a month ;)! I have been experimenting with this a bit lately, and I thought it'd be interesting to regroup and see where we're at. How has your experimenting gone? I was quite inspired by the things you've outlined here for the mocking, so I've built on those ideas when testing things. I'll explain what I've tried by this snippet of code:
query SomeQuery {
viewer {
id
firstName
lastName
allDimensions(first: 2) {
pageInfo {
hasNextPage
endCursor
}
edges {
cursor
node {
id
name
active
}
}
}
}
}
queryMock.mockQuery(
autoMock({
name: 'SomeQuery',
data: {
viewer: {
firstName: 'Gabriel',
allDimensions: {
pageInfo: {
endCursor: 'some-end-cursor'
},
$alterNode: allDimensions => {
mockHelpers.changeConnectionNode(allDimensions, 0, {
name: 'First node',
active: false
});
}
}
}
}
})
) {
"viewer": {
"id": "VXNlcjoyOTQ5OTIxOTkwNjU3NDQy",
"firstName": "Gabriel",
"lastName": "Hello World",
"allDimensions": {
"pageInfo": {
"hasNextPage": false,
"endCursor": "some-end-cursor"
},
"edges": [
{
"cursor": "Hello World",
"node": {
"id": "RGltZW5zaW9uOjc2NzQwOTc3MDQxMDM4Mg==",
"name": "First node",
"active": false
}
},
{
"cursor": "Hello World",
"node": {
"id": "RGltZW5zaW9uOjgxOTk3NDEyODk3ODU2MzI=",
"name": "Hello World",
"active": true
}
}
]
}
}
} This resolves everything in As you can see I'm experimenting with an What are your thoughts on something like this? Is it close to what you've tried? Naturally there's quite a few things left to solve (I've basically had to fork and alter |
Hey @zth, Apologies for being silent this week. I wanted to spend more time experimenting with auto-mocking before replying. I agree that there are still things to be figured regarding merging arrays. In your example, it seems that you are able to match the size of the On my end, I currently solved this issue by tweaking queryMock.mockQuery(
name: 'SomeQuery',
customHandler: autoMock({
viewer: {
firstName: 'Gabriel',
allDimensions: (obj, {first}) => ({
pageInfo: {
endCursor: 'some-end-cursor'
},
edges: new MockList(first, (obj, {arrayIndex}) => {
name: arrayIndex === 0 ? "First node" : undefined,
active: arrayIndex === 0,
})
})
}
})
) The main benefit of this pattern is that we keep the mocking API consistent, hence simple. All functions are In case we don't need to make queryMock.mockQuery(
name: 'SomeQuery',
customHandler: autoMock({
viewer: {
firstName: 'Gabriel',
allDimensions: {
pageInfo: {
endCursor: 'some-end-cursor'
},
edges: [
{
name: 'First node',
active: true,
},
{}
]
})
}
})
) Objects in arrays are merged with the base mock resolvers like any other regular types. You only specify the properties you need and the rest will be provided by the base mock. Btw I ended up rebuilding a So far I've been trying to tweak |
Hey @jnak! That's great! If we can deep merge the objects too even in the arrays (and not replace) I like your idea a whole lot better than that awkward I actually wasn't able to make it return the correct amount in the list, it's just a coincidence the query says 2 and I return 2. I worked some on that but I needed to change quite a lot in I look forward to hearing what you come up with! I think this feature is really great, maintaining real mocks at scale really isn't feasible sadly, so I'm very interested in seeing this work out. I'll see when I get the time to continue experimenting myself too. |
Hey @zth, After careful consideration I decided to rebuild the mocking functionality from scratch. I'm planning to write a detailed explanation of why I didn't use For now, I can share the main design decisions behind my implementations:
I haven't written documentation yet so for now you can look at the 40 or so test cases. I tried to cover all the different functionalities of the library. There are also detailed flow types that you might find useful. The merging logic is the following:
We're going to be dog fooding this internally at my company from my personal Github branch. Once we feel comfortable with it, we'll probably publish this as a separate Npm package in case other people want to use it with a different testing library. I realize some of my decisions are quite opinionated and might not be a good fit for everyone. That being said, I think it's big step forward from You can start using it by wrapping const server = mockServer(schemaDefinition, baseMocks);
...
function autoMock(queryMock) {
return async function autoMockCustomHandler(request, config) {
const results = await server(config.query, config.variables, queryMock);
return [200, results];
};
}
...
mock.mockQuery({
...
customHandler: autoMock({...})
}) Here is the link to my branch. Let me know what you think! |
I think this sounds just great and the right way to go with rewriting the mocking functionality. I haven't had time to test it yet, but I very much look forward to. I'll get back to you here as soon as I do. Great work, looking forward to continuing this! |
Quick update. I've spent a couple of more days polishing the library, fixing some edge cases, adding docs and setting a new repo for it. I'll make the repo public once the legal department of my company figure out the licence (most likely MIT). Hopefully early next week. |
Sounds great! I look forward to seeing the lib. I haven't had a chance to test your branch properly yet unfortunately, but I look forward to testing the lib. |
Hey @jnak! Just checking in, did you move forward with the lib? |
Hey @zth, yes my company finally figured out the licences for open source projects. I was planning to publish it and add proper documentation next week but I just made it public on github (repo) in case you want to take a sneak peek today. There is no doc yet but the tests are pretty thorough. Let me know if you run into any issues. |
Just a heads up that I pushed the library to |
Hey @jnak ! This looks really cool, lets continue the mocking-related discussion in your repo. |
Hello,
I've started using this library in our tests and I really like it so far. My main concern at this point is that we have to repeatedly mock data that is un-essential for a given test case. It is quite verbose and it's challenging to keep all nested fragment queries in sync with all the test cases across different files.
I'd like to be able to only specify core data for a given test and generate realistic mocks for the rest of the data. You can read more about this approach here.
To achieve this, I was thinking of adding support for async customHandler function so we can potentially call a graphql schema resolver function. Do you think that's the right approach? If so, I'd very happy to send a PR.
Thanks!
J
The text was updated successfully, but these errors were encountered: