actions-on-google / actions-on-google-testing-nodejs

Apache License 2.0
75 stars 18 forks source link

Provide action package and get coverage results #21

Closed yoichiro closed 6 years ago

yoichiro commented 6 years ago

Fix #16

What is this pull request

To provide a new feature to calculate a coverage rate and output it. This feature was proposed at #16.

How to use

  1. Export a ZIP file which has information of your agent from Dialogflow Console.
  2. Pass the file path to the 2nd argument of the ActionsOnGoogleAva constructor.
  3. Call the reportIntentCoverage function in the test.after callback handler.

The code to pass the zip file path is like the following:

const action = new ActionsOnGoogleAva(require('...'), {
    actionPackage: '<YOUR_ZIP_FILE_PATH>'
});

The dialogflowZip can also be used instead of the actionPackage.

The code to call the reportIntentCoverage function is like the following:

const test = require('ava');
...
test.after(() => {
    return action.reportIntentCoverage();
});

Then, you can see the coverage report after executing all tests as like the following:

11.76% (2/17) of intents covered in your tests.

How to design and implement

Basically, I would like to design logics as the following policies:

ActionsOnGoogle class provides an information about coverage

Currently, the ActionsOnGoogle class provides only assistant response by the send() function. The assistant response is represented by the AssistResponse interface. BTW, the intent ID matched by the Dialogflow is included into the response from the action. But, the current logic of the send() function ignores the intent ID.

To return both the assist response and the matched intent ID, I would like to create a new function called doSend(). The doSend() function returns both the assistant response and the matched intent ID. And, I would like to change the original send() logic to call the new doSend() function. The new send() function returns the assist response only and discards the matched intent ID.

The signature of the new doSend function is like the following:

interface AdditionalResponse {
    matchedIntentId?: string,
}

interface SendResponse {
  assistResponse: AssistResponse
  additionalResponse: AdditionalResponse
}

doSend(phrase: string): Promise<SendResponse> {
  ...
}

Collect coverage information and calculate it

Currently, the ActionsOnGoogleAva class overrides the send() function where the ActionsOnGoogle parent class. But, to get the coverage information, it is necessary to call the doSend() function. In the orverrided send() function, call the doSend(), and store the instance variable the intent ID included in the additional response. As the result, the ActionsOnGoogleAva class can collect all matched intent IDs from the all responses.

send(input: string) {
  return super.doSend(input)
    .then((res: SendResponse) => {
      const {assistResponse, additionalResponse} = res
      // Store the additionalResponse.matchedIntentId to the instance variable
      return Promise.resolve(assistResponse)
    })
}

Load the Dialogflow ZIP file and calculate the coverage result

I would like to add a feature to load the specified Dialogflow ZIP file and calculate the coverage result from the matched intent IDs.

To parse the zip file, I would like to use unzip library. I add the _loadIntentsFromDialogflowZip() function to load the zip file and parse it. In the process, parse all entries in the intents/ , and retrieve all intent IDs.

Then, in the new _calculateIntentCoverage() function, calculate the coverage result, and output it.

Note that the ActionsOnGoogleAva class cannot know the timing at all tests finished. In the current my idea, I want to request to write the code to call the new reportIntentCoverage() function in the test.after() callback function on the test code which developers write (refer to the How to use).

Fleker commented 6 years ago

Wow it's impressive that you put this together so quickly. I'm pretty interested to start trying it out.

yoichiro commented 6 years ago

@Fleker @atulep I wrote both the implementation code and test code. Could you review them?

yoichiro commented 6 years ago

I intend to write this usage to the README.md file after reviewing diffs of this pull request.

atulep commented 6 years ago

Hey @yoichiro,

Sorry for delayed response.

Thank you so much for submitting this implementation. However, we have other priorities as far as working on the library now, and we're afraid that adding new features will be costly for us now. As such, we'd like to put a freeze on new features for some time. We will still accept any bug fixes though.

There may have been misunderstanding between us. The issues we have created were intended to be more of open-ended questions, meaning we didn't quite agree upon integrating them into the library. That's said, I want to reiterate that we are very thankful for the work you've been putting into helping us shape the library.

We're going to keep your implementation, and try keep the library compatible with them, so we can reuse the code later. In any case, your design is good and we'll refer back to it should we decide to integrate this feature into the library.

yoichiro commented 6 years ago

I talked with @atulep about this directly, and I could understand the situation and reason. I close this pull request, but we can reopen this if necessary.