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

Add debugTestAtCursor command #2059

Merged
merged 5 commits into from
Feb 4, 2019

Conversation

Ashiroq
Copy link
Contributor

@Ashiroq Ashiroq commented Oct 28, 2018

Closes #1088

Adds command for debugging test at the current position of cursor similar to "debug test" codeLens.
By default, the command is hidden in the context menu, if codeLens is enabled just like with testAtCursor command.

Let me know if I can improve this :)

@msftclas
Copy link

msftclas commented Oct 28, 2018

CLA assistant check
All CLA requirements met.

Copy link
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.

Thanks for the PR @Ashiroq!

There is a lot of pre-processing being done in https://github.com/Microsoft/vscode-go/blob/0.6.93/src/goRunTestCodelens.ts#L89 that is missing here.

I would suggest to refactor that code so that it can be used here.

Also, I see that this is your first contribution to vscode-go. Welcome & Thanks!
I hope that you have registered both for the Microsoft Hacktoberfest and the one from Digitalocean

Happy Coding!

@vladbarosan
Copy link
Contributor

vladbarosan commented Jan 28, 2019

@Ashiroq I have some fixes at #2281 , can you give me permission to push to your branch ? @jhendrixMSFT , @ramya-rao-a ( getting permission denied when trying to push to the branch so assuming the permission was disabled )

@Ashiroq
Copy link
Contributor Author

Ashiroq commented Jan 28, 2019

I sent you an invitation to collaborators.

@vladbarosan
Copy link
Contributor

@Ashiroq I managed to update the branch. @ramya-rao-a i tested the the suite scenario for both the codelens and the command and they work fine.

}).then(null, err => {
console.error(err);
});
const isMod = await isModSupported(editor.document.uri);
Copy link
Contributor

@vladbarosan vladbarosan Jan 30, 2019

Choose a reason for hiding this comment

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

@ramya-rao-a here I have a question. previously the testConfig was saved to the global lastTestConfig without the isMod property. was that intentional or by accident ?

Copy link
Contributor

@vladbarosan vladbarosan Jan 30, 2019

Choose a reason for hiding this comment

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

i presume it was intended to be saved since the call to running previous test used it. if so then its fine since i just made it immutable but still is saved ( otherwise it should be cloned instea of just assigned )

@vladbarosan
Copy link
Contributor

@jhendrixMSFT , @ramya-rao-a the metalinter test seems to be failing although from looking at it doesn't seem related to anything i've changed. Any idea why ?

@jhendrixMSFT
Copy link
Member

@vladbarosan There was a new release of the metalinter v3.0.0 which has breaking changes. How about you update travis to pin to v2.0.12 until we have time to react to these changes?

@vladbarosan
Copy link
Contributor

@jhendrixMSFT doesn't seem to have helped :(

@vladbarosan
Copy link
Contributor

vladbarosan commented Jan 31, 2019

@jhendrixMSFT ok figured it out, it seems not only that the new version broke us but also the new install script version ( probably because it seems to be removing some checkers ).

As all our CI is blocked as of now, I locked to the previous script version for now and tracking the issue here #2292

Copy link
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.

As part of #2193, we now show test coverage even if test fails. So, I pushed a commit accordingly.

This was awesome work, thanks @vladbarosan!

@ramya-rao-a ramya-rao-a merged commit bcc6407 into microsoft:master Feb 4, 2019
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.

5 participants