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

Add a command to execute the previous test#478

Merged
ramya-rao-a merged 1 commit intomicrosoft:masterfrom
ironcladlou:test-previous
Sep 28, 2016
Merged

Add a command to execute the previous test#478
ramya-rao-a merged 1 commit intomicrosoft:masterfrom
ironcladlou:test-previous

Conversation

@ironcladlou
Copy link
Copy Markdown
Contributor

Add a go.test.previous command which runs the last test which was
executed.

@ironcladlou
Copy link
Copy Markdown
Contributor Author

@lukehoban do you have any idea what's up with those Travis failures?

@ramya-rao-a
Copy link
Copy Markdown
Contributor

ramya-rao-a commented Sep 15, 2016

@ironcladlou The test for gometalinter tries to set the user settings for linttool.

let config = vscode.workspace.getConfiguration('go');
config['lintTool'] = 'gometalinter';

which fails with the latest version of VSCode as you can no longer update config that way.

Interestingly the comment for the above config in vscode.d.ts says it is a readonly dictionary, though it has been writable till the last release

There have been changes to the configuration service in VS Code in August and more to come in September
I'd say we comment the test case for now until vscode comes up with the right way to set the configs

Logged #479

Comment thread src/goMain.ts

ctx.subscriptions.push(vscode.commands.registerCommand('go.test.previous', () => {
let goConfig = vscode.workspace.getConfiguration('go');
testPrevious();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you intend to pass testTimeout here as in other test commands? Or to not use goConfig?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually - looks like you are intentionally not passing testTimeout - so should drop the goConfig line.

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.

I intentionally disregard any settings as the intent of the command is to repeat the last test exactly as it was previously executed. I can imagine how you might want your timeout setting to override whatever's stored, but I wanted to keep it simple. Thoughts?

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.

Removed the config lookup for now.

Comment thread src/goTest.ts
* Runs the previously executed test.
*/
export function testPrevious() {
let editor = vscode.window.activeTextEditor;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does an editor need to be active in this case? It sounds like the semantics for this command are not tied to what is in the current editor (if there is one). It just executes whatever the last Go test was that was invoked.

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.

You're right, I'll remove that check.

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.

Removed the editor check.

Copy link
Copy Markdown
Member

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

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

A couple of cleanup things but the new command looks good.

@ironcladlou
Copy link
Copy Markdown
Contributor Author

@ramya-rao-a, thanks for tracking down that Travis issue!

@ironcladlou
Copy link
Copy Markdown
Contributor Author

Added a small commit to rename the title of the command to something more auto-complete friendly (as a complement to #495).

@ironcladlou
Copy link
Copy Markdown
Contributor Author

Squashed commits.

@ironcladlou
Copy link
Copy Markdown
Contributor Author

Will this be merged?

@ramya-rao-a
Copy link
Copy Markdown
Contributor

Sure, I tested your changes and they look good.

Could you trigger another travis runs so that the tests are all green? The last time it failed due to an upstream issue with vscode-debugadapter

@ironcladlou
Copy link
Copy Markdown
Contributor Author

@ramya-rao-a Thanks! How do I trigger a new build without making some dummy commit?

Add a `go.test.previous` command that runs the last test which was
executed.
@ramya-rao-a ramya-rao-a merged commit 02d05be into microsoft:master Sep 28, 2016
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.

4 participants