Add support for gotests "https://github.com/cweill/gotests" #489

clamoriniere · 2016-09-19T20:15:45Z

Add support for gotests "https://github.com/cweill/gotests"

Implements three new commands that use gotests:

go.test.generate.package: generates all unit-test squeletons for
the current package.
go.test.generate.file: generates all unit-test squeletons for
the current file.
go.test.generate.function: generates unit-test squeleton for the
selected function in the current file.

msftclas · 2016-09-19T20:15:48Z

Hi @cedriclam, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

msftclas · 2016-09-19T20:22:09Z

@cedriclam, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

lukehoban · 2016-09-25T19:40:19Z

src/goTests.ts

+	});
+}
+
+function testEditor() {


Add return type.

lukehoban · 2016-09-25T19:40:27Z

src/goTests.ts

+			symbols.filter(sym =>
+				sym.kind === vscode.SymbolKind.Function)
+		);
+}


Newline at EOF

lukehoban · 2016-09-25T19:44:06Z

src/goTests.ts

+	let editor = testEditor();
+	let file = editor.document.uri.fsPath;
+	generateTests({dir: file});
+	vscode.window.showInformationMessage('gotests: unit-tests generated properly for file:' + path.basename(file));


Perhaps "gotests: Unit tests generated for file: "?

lukehoban · 2016-09-25T19:49:59Z

package.json

@@ -102,6 +102,21 @@
        "description": "Displays test coverage in the current package."
      },
      {
+        "command": "go.test.generate.package",
+        "title": "Go: Generates unit-tests (package)",
+        "description": "Generates unit-tests for the current package"


change done in all files

lukehoban · 2016-09-25T19:50:25Z

README.md

@@ -19,6 +19,7 @@ This extension adds rich language support for the Go language to VS Code, includ
 - Build-on-save (using `go build` and `go test`)
 - Lint-on-save (using `golint` or `gometalinter`)
 - Format (using `goreturns` or `goimports` or `gofmt`)
+- Generate unit-test squeleton (using `gotests`)


"Generate unit tests"

Change done.

clamoriniere · 2016-09-27T19:28:55Z

@lukehoban thanks for your code review.
Let me know if you think that I need to change something else.

ramya-rao-a

Nice feature. I have added a few comments to get a clean end-to-end experience.

ramya-rao-a · 2016-09-28T20:42:31Z

src/goTests.ts

+}
+
+export function generateTestCurrentFile() {
+	let editor = testEditor();


When no active editor is found, then return. Don't call generateTests(). Else you will end up with 1 info box for "no editor" and 1 error box for "running the command ... failed". Same applies to the other 2 generate.. methods as well

you're right. I will add a check on the editor

ramya-rao-a · 2016-09-28T20:45:00Z

src/goTests.ts

@@ -0,0 +1,113 @@
+/*---------------------------------------------------------


Name this file "goGenerateTests" maybe? Because we already have a file goTest.ts and another go.test.ts. We can it keep the intent of the file clear this way

ramya-rao-a · 2016-09-28T20:47:57Z

src/goTests.ts

+	let editor = testEditor();
+	let file = editor.document.uri.fsPath;
+	generateTests({dir: file});
+	vscode.window.showInformationMessage('gotests: Unit tests generated for file:' + path.basename(file));


Show this message only after the promise returned from generateTests() is completed

Same applies to the other 2 methods as well

ramya-rao-a · 2016-09-28T21:15:12Z

src/goInstallTools.ts

@@ -24,7 +24,8 @@ let tools: { [key: string]: string } = {
 	'go-symbols': 'github.com/newhook/go-symbols',
 	'guru': 'golang.org/x/tools/cmd/guru',
 	'gorename': 'golang.org/x/tools/cmd/gorename',
-	'goimports': 'golang.org/x/tools/cmd/goimports'
+	'goimports': 'golang.org/x/tools/cmd/goimports',
+	'gotests': 'github.com/cweill/gotests'


This didn't work for me. The source code gets downloaded but the tool doesn't get dropped in the $GOPATH/bin. After looking at the source code structure, looks like there is another "gotests" folder.
This needs to be updated to 'github.com/cweill/gotests/gotests'

Make the change in the README as well

I will modify by: github.com/cweill/gotests/gotests/... since it is what they are saying in the doc.

ramya-rao-a · 2016-09-28T21:20:42Z

src/goTests.ts

+					return resolve(null);
+				}
+				if (err) {
+					return reject('Cannot generate test due to errors: ' + stderr);


This error doesn't bubble up to the user

ramya-rao-a · 2016-09-28T21:27:53Z

package.json

@@ -102,6 +102,21 @@
        "description": "Displays test coverage in the current package."
      },
      {
+        "command": "go.test.generate.package",
+        "title": "Go: Generates unit tests (package)",


Having parenthesis/brackets breaks autocomplete while typing commands in the command pallete.
#495 fixed this for the test commands
Can you update the title for your commands as well?
example: Go: Generate unit tests for package/file/function

ramya-rao-a · 2016-09-28T21:29:40Z

.travis.yml

@@ -31,6 +31,7 @@ install:
  - go get -u -v github.com/newhook/go-symbols
  - go get -u -v golang.org/x/tools/cmd/guru
  - go get -u -v github.com/alecthomas/gometalinter
+  - go get -u -v github.com/cweill/gotests


you don't need to add this to travis unless you have some unit tests. Could you add some unit tests?

I added unit tests

ramya-rao-a · 2016-09-28T21:30:52Z

src/goTests.ts

+	func?: string;
+}
+
+function generateTests(conf: Config): Thenable<vscode.WorkspaceEdit> {


Why return WorkspaceEdits?

I didn't know much about the vscode internals... I done a copy/past on the gotest plugin. I will check what it is better in my case.

ramya-rao-a · 2016-09-28T21:36:15Z

Also, this worked for me in Mac but not Windows. In Windows, gotests returned with this error

clamoriniere · 2016-09-29T20:22:15Z

@ramya-rao-a, thanks for you code review. I done your proposed changes.

about gotests failing on windows environment, did you try to run directlygotests with the command line? Maybe it is because you installed with the command: go get -u github.com/cweill/gotests/gotests instead of go get -u github.com/cweill/gotests/...

I haven't a windows environment yet but I will try next week.

Thanks again for your help.

clamoriniere · 2016-10-03T20:40:20Z

Hello @ramya-rao-a, I was able to test on a windows environment my last pull request version and it works fine.
env: Windows 7, vscode v1.5.3

clamoriniere · 2016-10-03T23:01:38Z

travis jobs failed due to an issue with the command "go get -u -v github.com/golang/lint/golint" and the environment go1.5

error: "../../golang/lint/lint.go:250: undefined: types.ImportMode"
link: https://travis-ci.org/Microsoft/vscode-go/jobs/164787607#L287

PR #510 should fix this issue

ramya-rao-a

Just 2 more things left, otherwise things look great!

ramya-rao-a · 2016-10-04T23:04:40Z

src/goGenerateTests.ts

+export function generateTestCurrentPackage(): Thenable<boolean>  {
+	let editor = testEditor();
+	if (!editor) {
+		vscode.window.showInformationMessage('gotests: No editor selected');


You already have a message being shown in line 116. You don't need this message again. I'd actually suggest to remove the function testEditor altogether and replace
let editor = testEditor()
with
let editor = vscode.window.activeTextEditor

Ok, I will do this change.

ramya-rao-a · 2016-10-05T18:34:41Z

src/goGenerateTests.ts

+					return resolve(false);
+				}
+				if (err) {
+					return reject('Cannot generate test due to errors: ' + stderr);


Turns out, this was failing for me in Windows because I had Go 1.5 on my Windows machine. The error message in such cases is not helpful. We should probably check for the Go version first and not run gotests at all if it is not supported and show a meaningful message like Generating tests using gotests is not supported in Go 1.5.

#509 adds a function for checking the Go version. You can either wait for that PR to get merged or copy over the getGoVersion function from there

Thanks, I will wait your PR integration.

@cedriclam my PR has been merged. Get latest from master, and you should have my changes

@ramya-rao-a: rebase done; travis builds look fine now.

Implements three new commands that use gotests: - go.test.generate.package: generates all unit-test squeletons for the current package. - go.test.generate.file: generates all unit-test squeletons for the current file. - go.test.generate.function: generates unit-test squeleton for the selected function in the current file.

msftclas added the cla-required label Sep 19, 2016

msftclas added cla-signed and removed cla-required labels Sep 19, 2016

clamoriniere force-pushed the feature/gotests branch from 69ebb69 to 74ee635 Compare September 19, 2016 20:25

lukehoban reviewed Sep 25, 2016

View reviewed changes

clamoriniere force-pushed the feature/gotests branch 2 times, most recently from 342c98d to 2eed87c Compare September 27, 2016 19:23

ramya-rao-a reviewed Sep 28, 2016

View reviewed changes

clamoriniere force-pushed the feature/gotests branch from 2eed87c to 4ecc64c Compare September 29, 2016 19:58

clamoriniere force-pushed the feature/gotests branch 6 times, most recently from ff397c2 to 8ce7301 Compare October 3, 2016 22:50

ramya-rao-a suggested changes Oct 5, 2016

View reviewed changes

clamoriniere force-pushed the feature/gotests branch 2 times, most recently from 86d9d2c to e4b0460 Compare October 6, 2016 18:36

clamoriniere force-pushed the feature/gotests branch from e4b0460 to 241c2df Compare October 6, 2016 18:40

ramya-rao-a approved these changes Oct 6, 2016

View reviewed changes

ramya-rao-a merged commit 1636cbe into microsoft:master Oct 6, 2016

		@@ -0,0 +1,113 @@
		/*---------------------------------------------------------

Add support for gotests "https://github.com/cweill/gotests" #489

Add support for gotests "https://github.com/cweill/gotests" #489

Conversation

clamoriniere commented Sep 19, 2016

msftclas commented Sep 19, 2016

msftclas commented Sep 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clamoriniere commented Sep 27, 2016

ramya-rao-a left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramya-rao-a Sep 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramya-rao-a commented Sep 28, 2016

clamoriniere commented Sep 29, 2016

clamoriniere commented Oct 3, 2016

clamoriniere commented Oct 3, 2016 • edited Loading

ramya-rao-a left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramya-rao-a Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramya-rao-a Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramya-rao-a Sep 28, 2016 •

edited

Loading

clamoriniere commented Oct 3, 2016 •

edited

Loading

ramya-rao-a Oct 5, 2016 •

edited

Loading

ramya-rao-a Oct 6, 2016 •

edited

Loading