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

Adding results directory UI option in vstest task #11789

Merged

Conversation

vagisha-nidhi
Copy link
Contributor

image

Copy link
Member

@PBoraMSFT PBoraMSFT left a comment

Choose a reason for hiding this comment

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

Please make the suggested changes.

@@ -22,6 +22,8 @@
"loc.input.help.tcmTestRun": "Test run based selection is used when triggering automated test runs from the test hub. This option cannot be used for running tests in the CI/CD pipeline.",
"loc.input.label.searchFolder": "Search folder",
"loc.input.help.searchFolder": "Folder to search for the test assemblies.",
"loc.input.label.resultsDirectory": "Test results directory",
"loc.input.help.resultsDirectory": "Folder to store test results. The vstest task cleans up results directory before test run. $(Agent.TempDirectory) is cleaned after each pipeline run. In case the default value is overriden, the directory will not be automatically cleaned. When results directory is provided in both task and runsettings, the runsettings will be given priority.",
Copy link
Member

Choose a reason for hiding this comment

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

When both are provided, task should win. Other inputs in the task typically override runsettings inputs, so this should be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making this change in the dtaexecutionhost

@@ -166,6 +166,14 @@ function getTestReportingSettings(inputDataContract : idc.InputDataContract) : i
inputDataContract.TestReportingSettings.TestRunTitle = tl.getInput('testRunTitle');
inputDataContract.TestReportingSettings.TestRunSystem = 'VSTS - vstest';

const resultsDir = path.resolve(tl.getVariable('System.DefaultWorkingDirectory'), tl.getInput('resultsFolder').trim());
Copy link
Member

Choose a reason for hiding this comment

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

common.testresults

{
inputDataContract.TestReportingSettings.TestResultsDirectory = resultsDir;
tl.debug("TestResultsFolder: " + resultsDir);
ci.publishEvent({ 'TestResultsFolderUi': resultsDir } );
Copy link
Member

Choose a reason for hiding this comment

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

temp or more than temp path
common.test or
defaultdir

push this property of the path as you can't directly push path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Is there a better way to add this?

@ShreyasRmsft ShreyasRmsft merged commit a81c7a5 into microsoft:master Nov 26, 2019

if (resultsDir.startsWith(tl.getVariable('Agent.TempDirectory') + 'TestResults'))
{
ci.publishEvent({ 'TestResultsFolderUi': '$(Agent.TempDirectory)/TestResults' } );
Copy link
Member

Choose a reason for hiding this comment

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

hey, can you add a customer feature name for this?

Other this event will have the same feature name as all the others and filtering on kusto won't be easy.

inputDataContract.TestReportingSettings.TestResultsDirectory = resultsDir;
tl.debug("TestResultsFolder: " + resultsDir);

if (resultsDir.startsWith(tl.getVariable('Agent.TempDirectory') + 'TestResults'))
Copy link
Member

Choose a reason for hiding this comment

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

did you verify if this has a slash at the end?

Copy link
Member

Choose a reason for hiding this comment

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

better to do path.join

}
else if (resultsDir.startsWith(tl.getVariable('Agent.TempDirectory')))
{
ci.publishEvent({ 'TestResultsFolderUi': '$(Agent.TempDirectory)' } );
Copy link
Member

Choose a reason for hiding this comment

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

please keep an eye out for formatting, use tslint

if (resultsDir.startsWith(tl.getVariable('Agent.TempDirectory') + 'TestResults'))
{
ci.publishEvent({ 'TestResultsFolderUi': '$(Agent.TempDirectory)/TestResults' } );
}
Copy link
Member

Choose a reason for hiding this comment

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

you can move this to a private function

leantk pushed a commit that referenced this pull request Dec 23, 2019
* Adding results directory UI option in vstest task

* Addressing comments

* Addressing PR comments

* Updating test agent

* Updating test agent version

* Updating relative directory and adding telemetry

* Update task.json

* Update task.json

* Updated help tect

* Updating res.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants