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

Apache License 2.0
75 stars 18 forks source link

Interactive mode. #17

Closed yoichiro closed 6 years ago

yoichiro commented 6 years ago

Fix the issue #9

I tried to implement the interactive prompt. Anyway, the screenshot is the following:

2018-08-07 14 21 42

How to use this

This is provided by the action-interactive executable command. The usage is the following:

$ action-interactive --help
Usage: action-interactive [Options] [Action] [Prompt]

Options:
  --help            Show help                                          [boolean]
  --credential, -c  Your credential file path                           [string]
  --locale, -l      Locale string                                       [string]

Examples:
  action-interactive -c ./credentials.json "my test app"

Developers can use the command right now, if the developer have installed this library for global. Otherwise, developers should be able to find the command at ./node_modues/.bin directory.

How to implement this

Fleker commented 6 years ago

Thanks a lot for doing this!

I am a little unsure of the name action-interactive. We were considering a more generalized tool action-test that would encompass interactivity but also generating the credentials through an action-test login command.

yoichiro commented 6 years ago

I am a little unsure of the name action-interactive. We were considering a more generalized tool action-test that would encompass interactivity but also generating the credentials through an action-test login command.

@Fleker Yes, actually, the name action-interactive was not a favorite to me as well... The name action-test looks good to me. As like there is the login sub command, it seems that a sub command interactive is better, I guess. For example,

$ action-test interactive -l en-US -c ./credentials.json "Personal recipe" "to find my recipes"

If you approve the idea above, I will change codes to rename the command name. Do you have other ideas?

yoichiro commented 6 years ago

@Fleker @atulep Thank you guys for the reviewing. I change some codes according to the comments.

yoichiro commented 6 years ago

@Fleker I add the action-test command, and also provide the interactive mode as the sub command as like the previous comment. Could you review the diff (0903d69) as well?

yoichiro commented 6 years ago

BTW, the current screenshot is the following:

2018-08-10 9 19 00
atulep commented 6 years ago

@yoichiro thanks! can you add unit-tests for new code?

yoichiro commented 6 years ago

@atulep Probably, yes. I'll try writing the test code. Give me a few days.

yoichiro commented 6 years ago

@atulep I added two test code files: action-test-command.test.ts and interactive.ts. And, I changed the file name from test.ts to actions-on-google-ava.test.ts. I was necessary to refactor some codes to be available the testings. The diff is 96d09d1. Could you review the commit?

atulep commented 6 years ago

@yoichiro,

I am sorry for delayed response. The tests look good to me.

However, I am having a difficult time using the interactive script. I tried:

  1. Running yarn build and doing node ./dist/actions-test interactive.js, but the JS file doesn't exist in the dist/
  2. I tried installing a tar of your version in a sample project, but npm fails to locate actions-test.js as well.

Do you have any ideas about it?

yoichiro commented 6 years ago

@atulep Sorry, I forgot to replace the script file path for the action-test command. I committed the fix code at ee117d9.

I need you to describe the structure about the new action-test command.

First, I would like to describe about how to use the action-test command, when users use the interactive prompt mode. After users install this actions-on-google-testing library, the action-test command is installed into the node_module/.bin/ directory. If the user install this to the global, the user will be able to use the action-test command directly, because the node_module/.bin directory should be included in the PATH environment variable.

When users type action-test --help in the terminal, they will see like the following output:

Usage: action-test <Command>

Commands:
  action-test interactive  Interactive mode

Options:
--help  Show help [boolean]

That is, the action-test command has an interactive sub-command. Users need to type action-test interactive <options> <action_name> <prompt> to start the interactive prompt mode. For example, when users type action-test interactive --help, they will see like the following output:

Usage: action-test interactive [Options] [Action] [Prompt]

Options:
  --help  Show help  [boolean]
  --credential, -c  Your credential file path  [string] [default: "./credentials.json"]
  --locale, -l  Locale string  [string] [default: "en-US"]

Examples:
  action-test interactive "Personal Recipe" "to find my recipes"

Running yarn build and doing node ./dist/actions-test interactive.js, but the JS file doesn't exist in the dist/

Thus, users is unnecessary to specify the interactive.js script file name. Instead, is necessary to specify the interactive sub command name. Actually, the action-test command is the main.js file. The main.js file is very simple, it has only calling the action-test-command.js. The action-test-command.js parses the command options and flags specified. If the sub command interactive is recognized, the main.js code calls the interactive.js file. The interactive.js file is the body for the interactive prompt mode.

I tried installing a tar of your version in a sample project, but npm fails to locate actions-test.js as well.

When I tested them after committing the ee117d9, the steps are the following:

  1. In the actions-on-google-testing-nodejs directory, type npm link to publish it for localy only.
  2. Create a new npm project for testing, then type npm link actions-on-google-testing.
  3. In the project directory for testing, type ./node_module/.bin/action-test interactive to launch the interactive prompt mode.

@atulep Could you test again?

yoichiro commented 6 years ago

@Fleker @atulep Could you have a time to review this?

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

@atulep I can understand that a building new feature is high cost and a priority is important. However, why did you require me to improve my codes and to write test codes so far? Also, why did you freeze a new feature after finishing these writing codes?

atulep commented 6 years ago

@yoichiro, at the time, we thought that we could add this feature, but because of your pull request, we are starting to look at it from a larger angle, trying to think what kind of features can be useful for the developers across all our tools.

yoichiro commented 6 years ago

@atulep Do you want to create a new tool/library? How will this tool be treated?

yoichiro commented 6 years ago

@atulep I'm deeply disappointed against the following comment: https://github.com/actions-on-google/actions-on-google-testing-nodejs/pull/17#issuecomment-420843024

I had been contributing continuously this testing library. Recent codes were written by me. But, this is not a problem. When I submitted this pull request, you reviewed my pull request, and you wished me to modify and add codes. I answered all them. When I finished them, you said to me that you freeze releasing all new features.

They said, "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. ". I can't agree. Why did you order the modifications and additions codes to me? Did you order me non-meaning things?

I believe that the testing library is so valuable. Also, I believe that new features I implemented are so valuable ideas as well. In fact, I'm using them during developing my actions. I'm thinking to release these new features as a new testing tool based on the current stable version 0.2.1...

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.