Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CustomExecution2 test and make start fire after onDidOpenTerminal #77839

Merged
merged 5 commits into from
Jul 24, 2019

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Jul 24, 2019

Part of #70978

cc @GabeDeBacker

@Tyriar Tyriar added this to the July 2019 milestone Jul 24, 2019
@Tyriar Tyriar requested a review from alexr00 July 24, 2019 00:36
let result: vscode.Task[] = [];
let kind: CustomTestingTaskDefinition = {
type: taskType,
customProp1: 'testing task one'
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if the test code could demonstrate the use of the value of 'customProp1'.

When I used this in my project, I ended up creating some wrapper classes around the use of virtual terminal process and custom executions so I could get the custom execution data (the properties) back into the execution of the extension code in a seemingly clean way.

When the code is all in one test like this, the "capturing" works just fine, but when you do things like put your task definition in one file, execution of the code in another, etc. it becomes a bit cumbersome.

I can point you to what I ended up doing if you want.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would this be as easy as just adding an assertion somewhere?

@Tyriar Tyriar merged commit 36e08fe into master Jul 24, 2019
@Tyriar Tyriar deleted the tyriar/task_vp_fix branch July 24, 2019 18:40
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
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.

3 participants