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

[feature request] Adding unit test case for modifyResponse function in Yurthub #800

Open
rambohe-ch opened this issue Apr 10, 2022 · 6 comments
Assignees
Labels
kind/feature kind/feature kind/good-first-issue kind/good-first-issue pinned

Comments

@rambohe-ch
Copy link
Member

What would you like to be added:
As discussed in the pull request #794 , I's more reliable to add some unit test case for modifyResponse function in yurthub. the code reference link is here: https://github.com/openyurtio/openyurt/blob/master/pkg/yurthub/proxy/remote/remote.go#L128-L203

Why is this needed:
modifyResponse function is entry for data filtering framework and local cache manager, so we need to make sure the stability for this function.

others
/kind feature

@rambohe-ch rambohe-ch added kind/feature kind/feature kind/good-first-issue kind/good-first-issue labels Apr 10, 2022
@xavier-hou
Copy link
Member

I have some questions here.

  1. I found that modifyResponse invokes some other functions, such as cachemanager.CacheResponse, which I guess might depend on specific storage. I would like to know if there is any way to test its correctness.
  2. Besides, I think some "local variables" such as resp.Request need to be tested as well since they are modified inside the function and seem to be cached. I found this difficult to cover with unit test.

Can anyone give me some advice?

@Congrool
Copy link
Member

It seems that we;ve already had the test of cachemanaer.CacheResponse in pkg/yurthub/cache_manager_test.go, so here we can foucus on modifyResponse itself.

How about write a fake cacheManager to instead? @HXCGIT

@stale
Copy link

stale bot commented Oct 18, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 18, 2022
@stale stale bot removed the wontfix label Oct 18, 2022
@rambohe-ch
Copy link
Member Author

/assign @rambohe-ch

@Himanshu372
Copy link

@rambohe-ch Is this issue still active, bcoz I think even the code has shifted to other - https://github.com/openyurtio/openyurt/blob/master/pkg/yurthub/proxy/util/remote.go#L83

I can take this up if the task is still needed to be done. Thanks

@rambohe-ch
Copy link
Member Author

rambohe-ch commented Apr 10, 2023

@rambohe-ch Is this issue still active, bcoz I think even the code has shifted to other - https://github.com/openyurtio/openyurt/blob/master/pkg/yurthub/proxy/util/remote.go#L83

I can take this up if the task is still needed to be done. Thanks

@Himanshu372 Very appreciate for taking this issue. i will assign this work to you.
/assign @Himanshu372

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature kind/feature kind/good-first-issue kind/good-first-issue pinned
Projects
Status: No status
Development

No branches or pull requests

4 participants