-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Users/shreyas r msft/input parser migration #7021
Users/shreyas r msft/input parser migration #7021
Conversation
Tasks/VsTest/models.ts
Outdated
@@ -1,4 +1,5 @@ | |||
import * as version from './vstestversion'; | |||
import { InputDataContract } from './inputdatacontract'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove this
Tasks/VsTest/taskinputparser.ts
Outdated
} | ||
console.log(tl.loc('dtaNumberOfAgents', inputDataContract.DistributionSettings.NumberOfTestAgents)); | ||
|
||
const distributionType = tl.getInput('distributionBatchType'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to dta
Tasks/VsTest/taskinputparser.ts
Outdated
const distributeByAgentsOption = tl.getInput('batchingBasedOnAgentsOption'); | ||
if (distributeByAgentsOption && distributeByAgentsOption === 'customBatchSize') { | ||
const batchSize = parseInt(tl.getInput('customBatchSizeValue')); | ||
if (!isNaN(batchSize) && batchSize > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to do thjis here?
Tasks/VsTest/taskinputparser.ts
Outdated
const rerunType = tl.getInput('rerunType') || 'basedOnTestFailurePercentage'; | ||
|
||
// hydra: unravel the nestings | ||
if (rerunType === 'basedOnTestFailureCount') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switching off rerun logic to dta executiion host?
Tasks/VsTest/distributedtest.ts
Outdated
@@ -31,8 +31,10 @@ export class DistributedTest { | |||
} | |||
|
|||
private publishCodeChangesIfRequired(): void { | |||
if (this.dtaTestConfig.tiaConfig.tiaEnabled) { | |||
const code = testSelector.publishCodeChanges(this.dtaTestConfig.tiaConfig, this.dtaTestConfig.proxyConfiguration, null, this.dtaTestConfig.taskInstanceIdentifier); | |||
if (this.inputDataContract.ExecutionSettings.TiaSettings.Enabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/VsTest/distributedtest.ts
Outdated
const envVars: { [key: string]: string; } = process.env; // This is a temporary solution where we are passing parent process env vars, we should get away from this | ||
this.inputDataContract.TestSelectionSettings.TestSourcesFile = this.createTestSourcesFile(); | ||
// Temporary solution till this logic can move to the test platform itself | ||
if (this.inputDataContract.UsingXCopyTestPlatformPackage) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seperate method
Tasks/VsTest/distributedtest.ts
Outdated
const envVars: { [key: string]: string; } = process.env; // This is a temporary solutio where we are passing parent process env vars, we should get away from this | ||
utils.Helper.addToProcessEnvVars(envVars, 'DTA.AccessToken', this.dtaTestConfig.dtaEnvironment.patToken); | ||
utils.Helper.addToProcessEnvVars(envVars, 'DTA.AccessToken', this.inputDataContract.AccessToken); | ||
this.inputDataContract.AccessToken = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setting it back to null. This is required
Tasks/VsTest/distributedtest.ts
Outdated
DistributedTest.removeEmptyNodes(inputDataContract); | ||
writeFileSync(inputFilePath, JSON.stringify(inputDataContract)); | ||
DistributedTest.removeEmptyNodes(this.inputDataContract); | ||
writeFileSync(inputFilePath, JSON.stringify(this.inputDataContract)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meaningful error? and telemetry
Tasks/VsTest/distributedtest.ts
Outdated
@@ -290,8 +165,8 @@ export class DistributedTest { | |||
|
|||
private cleanUpDtaExeHost() { | |||
try { | |||
if (this.testSourcesFile) { | |||
tl.rmRF(this.testSourcesFile); | |||
if (this.inputDataContract.TestSelectionSettings.TestSourcesFile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really require as now we are using AGENT.TEMP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this
Tasks/VsTest/distributedtest.ts
Outdated
let sourceFilter = tl.getDelimitedInput('testAssemblyVer2', '\n', true); | ||
|
||
if (this.inputDataContract.TestSelectionSettings.TestSelectionType.toLowerCase() !== 'testassemblies') { | ||
sourceFilter = ['**\\*', '!**\\obj\\*']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how many times we are searching? I am assuming we are only searching once from the task and passing it to DTAExecutionHost all the list of file inputs. for both testplan/test assembly sceanrio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had been doing this replication from some time now. Will have to go ahead and remove it from the C# layer
Tasks/VsTest/task.json
Outdated
@@ -16,8 +16,8 @@ | |||
"author": "Microsoft Corporation", | |||
"version": { | |||
"Major": 2, | |||
"Minor": 5, | |||
"Patch": 6 | |||
"Minor": 134, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??
change it 2.6.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was considering setting it to sprint number. Just a thought
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for sprint number too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will ask shiva and finalize
@@ -106,6 +107,109 @@ export class TestSelectorInvoker { | |||
return output.code; | |||
} | |||
|
|||
public publishCodeChangesInDistributedMode(inputDataContract: inputdatacontract.InputDataContract): number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get it reviewed with praga
|
||
taskinputparser.logWarningForWER(tl.getBoolInput('uiTests')); | ||
|
||
inputDataContract.UseNewCollector = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiaNewCollector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
temp var. leaving it as it is for now. Will remove in the next testselector pr
Tasks/VsTest/inputparser.ts
Outdated
inputDataContract.TeamProject = tl.getVariable('System.TeamProject'); | ||
inputDataContract.VstestTaskInstanceIdentifier = uuid.v1(); | ||
|
||
inputDataContract.UseVsTestConsole = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/VsTest/inputparser.ts
Outdated
inputDataContract.RunIdentifier = getRunIdentifier(); | ||
|
||
//hydra: do we need | ||
//dtaEnvironment.dtaHostLogFilePath = path.join(tl.getVariable('System.DefaultWorkingDirectory'), 'DTAExecutionHost.exe.log'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} | ||
|
||
function getTestSelectionInputs(inputDataContract : idc.InputDataContract) : idc.InputDataContract { | ||
inputDataContract.TestSelectionSettings = <idc.TestSelectionSettings>{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check this works in Node 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/VsTest/inputparser.ts
Outdated
if (vsTestLocationMethod === utils.Constants.vsTestVersionString) { | ||
const vsTestVersion = tl.getInput('vsTestVersion'); | ||
if (utils.Helper.isNullEmptyOrUndefined(vsTestVersion)) { | ||
console.log('vsTestVersion is null or empty'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} else { | ||
// hydra: should it be full path or directory above? | ||
inputDataContract.VsTestConsolePath = tl.getInput('vsTestLocation'); | ||
console.log(tl.loc('vstestLocationSpecified', 'vstest.console.exe', inputDataContract.VsTestConsolePath)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- VsTools path -> check what's the path
- Vstest.console.exe path -> vs 2015 [IDE]
- vstest.console.exe path -> vs 15.5
- vstest.console.exe path -> 15.6 > [\Extensions\TestWindow]
Tasks/VsTest/inputparser.ts
Outdated
const distributionType = tl.getInput('distributionBatchType'); | ||
|
||
if (distributionType && distributionType === 'basedOnTestCases') { | ||
inputDataContract.DistributionSettings.TestCaseLevelSlicingEnabled = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the naming change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/VsTest/inputparser.ts
Outdated
console.log(tl.loc('runInParallelInput', inputDataContract.ExecutionSettings.AssemblyLevelParallelism)); | ||
|
||
// hydra: valid only for console flow | ||
//inputDataContract.ExecutionSettings.runTestsInIsolation = tl.getBoolInput('runTestsInIsolation'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added back
Tasks/VsTest/inputparser.ts
Outdated
|
||
// hydra: console flow only | ||
//testConfiguration.otherConsoleOptions = tl.getInput('otherConsoleOptions'); | ||
//console.log(tl.loc('otherConsoleOptionsInput', testConfiguration.otherConsoleOptions));\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/VsTest/inputparser.ts
Outdated
inputDataContract.ExecutionSettings.TiaSettings.FileLevel = getTIALevel(tl.getVariable('tia.filelevel')); | ||
inputDataContract.ExecutionSettings.TiaSettings.SourcesDirectory = tl.getVariable('build.sourcesdirectory'); | ||
inputDataContract.ExecutionSettings.TiaSettings.FilterPaths = tl.getVariable('TIA_IncludePathFilters'); | ||
// hydra: inputDataContract.ExecutionSettings.TiaSettings.runIdFile = path.join(os.tmpdir(), uuid.v1() + '.txt'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless testselector tool code is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not being used today.
tl.debug('Searching for latest Visual Studio'); | ||
const vstestconsole15Path = versionfinder.getVSTestConsole15Path(); | ||
if (vstestconsole15Path) { | ||
vsTestVersion = '15.0'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check for manual vstest.console.exe path where cx can give full path or dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in the invoking function above
@@ -106,6 +107,109 @@ export class TestSelectorInvoker { | |||
return output.code; | |||
} | |||
|
|||
public publishCodeChangesInDistributedMode(inputDataContract: inputdatacontract.InputDataContract): number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Help me understand this. Why did we create a new invoker for distributed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because inputs need to come from inputdatacontract which currently does not exist for single agent flow.
No description provided.