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.

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

Suggestion for Improving Test Coverage of client/utils.ts #2980

Closed
yasuaki640 opened this issue Jun 16, 2024 · 1 comment · Fixed by #2985
Closed

Suggestion for Improving Test Coverage of client/utils.ts #2980

yasuaki640 opened this issue Jun 16, 2024 · 1 comment · Fixed by #2985
Labels
enhancement New feature or request.

Comments

@yasuaki640
Copy link
Contributor

What is the feature you are proposing?

In utils.test.ts it looks like early return statements are not executed in the test code.

https://app.codecov.io/gh/honojs/hono/blob/main/src%2Fclient%2Futils.ts#L40

As a suggested fix, why not modify utils.test.ts as follows?

 describe('deepMerge', () => {
   it('should return the source object if the target object is not an object', () => {
     const target = null
-    const source = { a: 1 }
+    const source = 'not object' as unknown as Record<string, unknown>

As another suggestion, if we assume that only objects are passed to the source argument of the deepMerge function, the conditional expression of the if statement could be as follows.

 export function deepMerge<T>(target: T, source: Record<string, unknown>): T {
-  if (!isObject(target) && !isObject(source)) {
+  if (!isObject(target)) {
     return source as T
   }

If this modification is necessary, I will make a PR.

@yasuaki640 yasuaki640 added the enhancement New feature or request. label Jun 16, 2024
@yusukebe
Copy link
Member

Hi @yasuaki640

I would prefer the former because I think the condition !isObject(target) && !isObject(source) is necessary.

If this modification is necessary, I will make a PR.

Please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants