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

fix(testing): create new session before recording #11693

Merged
merged 2 commits into from
Apr 4, 2022

Conversation

laurentlp
Copy link
Contributor

This PR fixes an issue where recording a scenario starting from the middle of a flow would result in the scenario always failing. To solve the issue, I see two possibilities.

  1. First, the one which this PR implements, is to reset or create a new session just before we start recording the scenario.
  2. We could simply save the state of the user's session before the recording starts and put this into the resulting scenario so that way we could restore the session before running the test. Since I see this option more as a feature than a fix, I opted for the simple one.

Closes: botpress/v12#1087

@linear
Copy link

linear bot commented Apr 4, 2022

DEV-1089 Testing module (botpress/botpress botpress/v12#1087)

Describe the bug
The testing module needs a rework.

  1. For bots that have flows with more than a few transitions, you need to think about manually resetting the conversation before starting a recording, otherwise the saved test doesn't work.

To Reproduce:

  1. Open the emulator, start a conversation with your bot. Stop in the middle of a flow.
  2. Go to the testing module, start recording
  3. Talk some more to the bot
  4. Stop recording and save
  5. Notice the interactions saved are exactly the ones recorded. If you select the play button, the test will fail, since the test starts from a new conversation to compare answers.

Expected behavior
Please validate: Emulator conversation is automatically reset for a valid scenario, or allow scenarios to start mid-flow from a defined checkpoint.

  1. There is a timeout set at one second. If transitions, or any custom action in between replies, take more than one second, the test run will fail. Similarly, the extensions module calls fails during a test run, which introduces additional delay.

To Reproduce:

  1. Easiest way to reproduce is in 12.10.5, before the flow fix. Make a bot or ask me about one that is big enough (in the 50 MB range)
  2. Record a scenario in the testing module
  3. Run the scenario
  4. In the network tab, notice all calls made to /mod/extensions are failing, one after the other. In a second, you will get a negative result because there is no reply to your first message.

Expected behavior
Testing should not be restricted by a timeout, let the test run.

Environment :

  • Botpress Version 12.10.6

Additional context
Features section : add a delete button for outdated scenarios. Maybe add a way to update the scenario without re-recording?

import { Preview, Scenario, Status } from '../../backend/typings'
import NoScenarios from './NoScenarios'
import ScenarioComponent from './Scenario'
import ScenarioRecorder from './ScenarioRecorder'
import style from './style.scss'

const WEBCHAT_READY_TIMEOUT = 10000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opted for a 10sec timeout since I simply don't want to be waiting indefinitely if something goes wrong but at the same time I want to give the webchat all the time it need to create the new session.

})

// Sends a request to the emulator to create a new session (new user, new socket connection, and new conversation)
win.postMessage({ action: 'new-session' }, '*')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same logic used in the debugged to create a new session (or reset it)

Copy link
Contributor

@samuelmasse samuelmasse left a comment

Choose a reason for hiding this comment

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

LGTM

@laurentlp laurentlp merged commit 05e0a86 into master Apr 4, 2022
@laurentlp laurentlp deleted the dev-1089-testing-module-botpressbotpress-3880 branch April 4, 2022 21:47
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.

Testing module
2 participants