Skip to content
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.

Sign up for GitHub

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

[issue-#6] Include flag as is (no case formatting) #7

Merged
merged 5 commits into from
Feb 3, 2022
Merged

[issue-#6] Include flag as is (no case formatting) #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e5abf3
Include options in mockFlags() in order to ignore case formatting. Re…
tboulis Jun 9, 2021
cb7e666
fix typo
tboulis Jun 9, 2021
801ea13
Include original flag as is without case formatting. Code review changes
tboulis Jan 13, 2022
9a0c897
Fix conflicts with master
tboulis Jan 13, 2022
ccbe851
Update README.md
tboulis Feb 1, 2022
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = {
## Usage
Use the only 3 apis for test cases:

* `mockFlags(flags: LDFlagSet)`: mock flags at the start of each test case.
`mockFlags(flags: LDFlagSet)`: mock flags at the start of each test case.

* `ldClientMock`: a jest mock of the [ldClient](https://launchdarkly.github.io/js-client-sdk/interfaces/_launchdarkly_js_client_sdk_.ldclient.html). All
methods of this object are jest mocks.
Expand Down
11 changes: 11 additions & 0 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ describe('main', () => {
expect(current['dev-test-flag']).toBeTruthy()
})

test('mock option without case formatting correctly', () => {
mockFlags({ DEV_test_Flag: true })

const {
result: { current },
} = renderHook(() => useFlags())

expect(current.DEV_test_Flag).toBeTruthy()
expect(current['DEV_test_Flag']).toBeTruthy()
})

test('mock asyncWithLDProvider correctly', () => {
expect(asyncWithLDProvider).toBeDefined()
})
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export const mockFlags = (flags: LDFlagSet) => {
const camel = camelCase(k)
result[kebab] = flags[k]
result[camel] = flags[k]
Copy link
Collaborator

@yusinto yusinto Jan 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the PR and apologies for the long wait. Instead of skipping formatting (or aliasing) we can add the original flag key as-is so the pristine value is returned in this function. This way we can support all cases without having to set any extra options:

export const mockFlags = (flags: LDFlagSet) => {
  mockUseFlags.mockImplementation(() => {
    const result: LDFlagSet = {}
    Object.keys(flags).forEach((k) => {
      const kebab = kebabCase(k)
      const camel = camelCase(k)
      result[kebab] = flags[k]
      result[camel] = flags[k]
      result[k] = flags[k] // Add the original flag key and value here
    })
    return result
  })
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's ok @yusinto , I've made the changes you suggested

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you @tboulis ! I'll go ahead and merge this now. I will make a release soon. Thanks again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One more comment before I can merge this @tboulis please. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure @yusinto !

result[k] = flags[k]
})
return result
})
Expand Down