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

fixed the assignment of mocked log functions in plugin_test.js #143

Merged
merged 3 commits into from
Feb 21, 2022

Conversation

dwoctor
Copy link
Contributor

@dwoctor dwoctor commented Feb 21, 2022

I was writing tests for a Kong JS Plugin, and encountered Error: function kong.log.err is not a valid PDK function whenever the plugin tried to interact with await kong.log.err("insert-error-here");.

After looking into the code, I found the mockFunctions in plugin_test.js where assigned under the index not the value of the elements in logFunctions.

I traced the issue to the for-in loop the returning the index not the value of the elements of logFunctions.

In this PR the assignment has been changed to use the name and not the index by using a basic for loop.

I hope these changes are good enough to be merged into the mainline.
If not the feedback would be appreciated.

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

Looks like the 14.x build failed, any ideas on what the issue is?

@CLAassistant
Copy link

CLAassistant commented Feb 21, 2022

CLA assistant check
All committers have signed the CLA.

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

I ran npm test on my local machine with node v14.19.0, all the tests came back passed. Maybe it was a glitch in the CI runner?

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

I think the issue is occurring on line 28 of test not line 30 as the CI output suggests. It looks to be the test is acting a bit non deterministically in regards to the sleep.

expect(mod.getLastStartInstanceTime()).toBeGreaterThan(start)

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

I think if await sleep(1000) is moved from line 24 to above line 14 the inconsistency is removed. The issue is the plugin is executing immediately because and the sleep is in the wrong place to get the desired behaviour.

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

I have added the changes to the pluing.test.js as part of this PR. The changes moves the sleep from line 24 to below line 12.

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

I think I need a reapprove to get the CI runners going again.

@fffonion
Copy link
Contributor

Looks good! Thanks for the quick fix for test as well! @dwoctor

@fffonion fffonion merged commit 1b4aa12 into Kong:master Feb 21, 2022
@dwoctor dwoctor deleted the patch-1 branch February 21, 2022 13:57
@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

@fffonion Thanks for approving and merging this change quickly. Is there going to be a release soon?

@fffonion
Copy link
Contributor

I will create a patch release tomorrow @dwoctor , it will be 0.5.3.

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 21, 2022

@fffonion Thanks.

@fffonion
Copy link
Contributor

@dwoctor it's now released!

@dwoctor
Copy link
Contributor Author

dwoctor commented Feb 23, 2022

@fffonion Thank you.

fffonion pushed a commit that referenced this pull request Oct 7, 2022
* fixed the assignment of mocked log functions in plugin_test.js

* removed semicolon to match style

* moved the sleep in pluing.test.js to get desired behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants