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

Considering receiverType on function test creation issue-2282#2987

Merged
ramya-rao-a merged 4 commits intomicrosoft:masterfrom
marcel-basel:issue-2282
Jan 26, 2020
Merged

Considering receiverType on function test creation issue-2282#2987
ramya-rao-a merged 4 commits intomicrosoft:masterfrom
marcel-basel:issue-2282

Conversation

@marcel-basel
Copy link
Copy Markdown
Contributor

The receiver type for a method is now considered. So, if there are >= 1 method names in the same file, then clicking "generate unit tests for function" creates test functions only for the selected method.

funcName like "(*Point).Move" will results in "gotests -w -only ^PointMove$ calls.

@marcel-basel marcel-basel changed the title Considering receiverType on function test creation Considering receiverType on function test creation issue-2282 Jan 17, 2020
Copy link
Copy Markdown
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Looks like this is your first PR contribution to this project @marcel-basel, Thanks & Welcome!

Comment thread src/goGenerateTests.ts Outdated
if (funcName.includes('.')) {
funcName = funcName.split('.')[1];
if (funcName.includes('.')) { // receiverType specified
let funcNameParts = funcName.match(/^\(\*?(.*)\)\.(.*)$/);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we are going to use a regular expression, then the if (funcName.includes('.')) check is redundant right?

Do we foresee the function name to include . in a non receiverType case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi Ramya, thanks for your fantastic work on the extension!

I removed the if (funcName.includes('.')) block. I don't see an other case where the function name would include . Do you?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nope, change looks good. I have pushed a commit to fix the linting errors, we are good to merge now

@ramya-rao-a ramya-rao-a merged commit 41591e3 into microsoft:master Jan 26, 2020
@marcel-basel
Copy link
Copy Markdown
Contributor Author

Thanks @ramya-rao-a, do you consider #2282 as closable?

@ramya-rao-a
Copy link
Copy Markdown
Contributor

I'll be closing it once I release an update with the fix.

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