Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Package suggestion for _test.go #1220

Merged
merged 13 commits into from
Sep 28, 2017
Merged

Package suggestion for _test.go #1220

merged 13 commits into from
Sep 28, 2017

Conversation

uudashr
Copy link
Contributor

@uudashr uudashr commented Sep 17, 2017

Assume the directory name is messaging

  1. File 'main.go' => suggestion package main
  2. File *_test.go => suggestion package messaging_test
  3. File *internal_test.go and rest of *.go => suggestion package messaging

@uudashr
Copy link
Contributor Author

uudashr commented Sep 17, 2017

@ramya-rao-a please check

@uudashr uudashr closed this Sep 22, 2017
@uudashr uudashr reopened this Sep 22, 2017
src/goSuggest.ts Outdated
}

if (goFilename.endsWith('internal_test.go')) {
return resolve(basename(dirname(filename)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you point me to any docs that talk about this as a standard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are practice to make a test package to test un-exported method when necessary.

No official docs but here:

  1. https://medium.com/@matryer/5-simple-tips-and-tricks-for-writing-unit-tests-in-golang-619653f90742
  2. https://blog.alexellis.io/golang-writing-unit-tests/

Hmm.. I found not too much:

$ find /usr/local/Cellar/go/1.9/libexec/src | grep internal_test.go
/usr/local/Cellar/go/1.9/libexec/src/net/http/transport_internal_test.go
/usr/local/Cellar/go/1.9/libexec/src/os/exec/internal_test.go
/usr/local/Cellar/go/1.9/libexec/src/strconv/internal_test.go
/usr/local/Cellar/go/1.9/libexec/src/time/internal_test.go

README.md Outdated
@@ -15,39 +12,43 @@ This extension adds rich language support for the Go language to VS Code, includ
- Quick Info (using `gogetdoc` or `godef`+`godoc`)
- Goto Definition (using `gogetdoc` or `godef`+`godoc`)
- Find References (using `guru`)
- Find implementations (using `guru`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge from master, there are a lot of changes there

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'm rebase from the master, seems not good rebase to an existing PR

Copy link
Contributor Author

@uudashr uudashr Sep 24, 2017

Choose a reason for hiding this comment

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

@ramya-rao-a Do you want me to re-create clean PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please, let's just do that. There is too much noise this PR.

@uudashr uudashr closed this Sep 24, 2017
@uudashr uudashr reopened this Sep 24, 2017
@uudashr
Copy link
Contributor Author

uudashr commented Sep 24, 2017

Also add package suggestion when the directory name formed by multiple words, ex:

  • go-expand-tilde -> tilde
  • go-spew -> spew
  • go-validator -> validator

@ramya-rao-a
Copy link
Contributor

@uudashr I have rebased from master. Can you check if everything is as you expected?

src/util.ts Outdated

export function readDir(path): Promise<string[]> {
return new Promise<string[]>((resolve, reject) => {
fs.readdir(path, (err, files) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of getting all files under the path and then checking for main.go, isn't it simpler to directly check for path + 'main.go' using fs.statSync(filePath).isFile() ?

@ramya-rao-a
Copy link
Contributor

@uudashr Also updated to use fs.stat to look for the main.go file instead of looking through files. Can you check if your test cases still hold good?

@uudashr
Copy link
Contributor Author

uudashr commented Sep 28, 2017

@ramya-rao-a fs.stat() such a great idea
All work as expected

@ramya-rao-a
Copy link
Contributor

@uudashr I also moved the whole pkg guessing logic to the utils file to make it re-usable as well as unit testable.

When you have some time, can you add unit tests for that function? For all the cases that you have in the comments?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants