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

Broken statement in core.ts when selecting the adapter in the spawn() function of version 4.9.0 #1984

Closed
etiennellipse opened this issue Jun 23, 2020 · 2 comments · Fixed by #2022
Assignees
Milestone

Comments

@etiennellipse
Copy link
Contributor

etiennellipse commented Jun 23, 2020

While trying to pinpoint an issue with using a custom adapter and worker within unit tests, I came upon a change that happened in #1938 on core.ts right here. It is in the spawn function of the core.ts file, when assigning the adapter variable.

Latest version, line 1093

This condition seems faulty:

const adapter = custom_adapter || (config && config.context && config.context.adapter) ? config.context.adapter : this.adapter;

It will never return the custom_adapter, if one was passed. Since the statement is now a ternary, the custom_adapter will not be returned.

As in this example:

true || (false) ? "1" : "2"

The returned value is "1", not true.

To get something similar to the previous behaviour, the line should look something like:

const adapter = custom_adapter || ((config && config.context && config.context.adapter) ? config.context.adapter : this.adapter);

I cannot test this since I have no idea how to pass a custom_adapter to the spawn function! I did not find any test code using it.

Context:

  • Botkit version: 4.9.0
@etiennellipse etiennellipse changed the title Broken statement in core.ts when selecting the adapter in the spawn() function Broken statement in core.ts when selecting the adapter in the spawn() function of version 4.9.0 Jun 23, 2020
@benbrown
Copy link
Contributor

Yikes! That is an unexpected behavior!!!

@benbrown benbrown self-assigned this Jul 23, 2020
@benbrown benbrown added this to the 4.9 milestone Jul 23, 2020
@benbrown
Copy link
Contributor

I fixed this and added some tests to make sure this doesn't regress. Thanks!!

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 a pull request may close this issue.

2 participants