Skip to content
This repository has been archived by the owner on Jul 12, 2019. It is now read-only.

Add optional parameter to createAppTester to customize storeKey #147

Merged
merged 3 commits into from
Apr 1, 2019
Merged

Add optional parameter to createAppTester to customize storeKey #147

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a84580
Add optional parameter to createAppTester to customize storeKey and u…
michellechu77 Mar 28, 2019
ceb56fd
Update variable name
michellechu77 Mar 29, 2019
f9de801
Update undefined to null
michellechu77 Apr 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export const tools: { env: { inject: (filename?: string) => void } };

// see: https://github.com/zapier/zapier-platform-cli/issues/339#issue-336888249
export const createAppTester: (
appRaw: object
appRaw: object,
storeKey: string
michellechu77 marked this conversation as resolved.
Show resolved Hide resolved
) => <T extends any, B extends Bundle>(
func: (z: ZObject, bundle: B) => Promise<T>,
bundle?: Partial<B> // partial so we don't have to make a full bundle in tests
Expand Down
5 changes: 3 additions & 2 deletions src/tools/create-app-tester.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const promisifyHandler = handler => {
};

// A shorthand compatible wrapper for testing.
const createAppTester = appRaw => {
const createAppTester = (appRaw, storeKey = null) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is good, but typically it's good to avoid null. It behaves weirdly with default args.

That said, you don't actually have to set a default here. The verbose version of this function signature is (appRaw, storeKey=undefined), but since js doesn't really enforce arguments at all, it's most clearly written as (appRaw, storeKey). Does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now that you likely copied the null from below. Honestly I shouldn't have written it like that in the first place. 😁

const handler = createLambdaHandler(appRaw);
const createHandlerPromise = promisifyHandler(handler);

Expand All @@ -46,14 +46,15 @@ const createAppTester = appRaw => {
bundle = bundle || {};

const method = resolveMethodPath(appRaw, methodOrFunc);
let myStoreKey = storeKey ? `testKey-${storeKey}` : `testKey-${method}-${randomSeed}`
Copy link
Contributor

Choose a reason for hiding this comment

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

though it's perfect for quick adhoc use, i'd probably name this variable something else. maybe customStoreKey.

That said, looking below, I'd probably move the ternary up here and rename that variable. So the whole thing would be

(appRaw, customStoreKey) => 
// ...
const storeKey = shouldPaginate(appRaw, method)
  ? storeKey
    ? `testKey-${storeKey}`
    : `testKey-${method}-${randomSeed}`
  : undefined;

// ...
  method,
  bundle,
  storeKey
};

if you go that route, remember to adjust the index.d.ts appropriately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, I like your suggested variable name better.


const event = {
command: 'execute',
method,
bundle,
storeKey: shouldPaginate(appRaw, method)
? // this key will be consistent across runs but unique to each test so we don't lose cursors
`testKey-${method}-${randomSeed}`
myStoreKey
: null
};

Expand Down