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 support for testconfig.json entries in MSTest #3572

Closed
Evangelink opened this issue Aug 15, 2024 · 3 comments · Fixed by #3872
Closed

Add support for testconfig.json entries in MSTest #3572

Evangelink opened this issue Aug 15, 2024 · 3 comments · Fixed by #3872

Comments

@Evangelink
Copy link
Member

Summary

Add support for testconfig.json entries.

Background and Motivation

Microsoft.Testing.Platform provides support for [appname].testconfig.json as a modern replacement for runsettings.

Proposed Feature

Detect if the config exists in the file and use it.

If the user is specifying the config both in the runsettings and testconfig.json let's fail and ask the user to specify in only one file.

Given we have a new file format, I think this is a good opportunity to refactor the settings into logical groups.

Here is the list of currently supported properties

AssemblyCleanupTimeout
AssemblyInitializeTimeout
AssemblyResolution
	<AssemblyResolution> <Directory path="D:\myfolder\bin\" includeSubDirectories="false"/> </AssemblyResolution>
CaptureTraceOutput
ClassCleanupLifecycle
ClassCleanupTimeout
ClassInitializeTimeout
DeleteDeploymentDirectoryAfterTestRunIsComplete
DeploymentEnabled
DeployTestSourceDependencies
EnableBaseClassTestMethodsFromOtherAssemblies
ForcedLegacyMode
MapInconclusiveToFailed
MapNotRunnableToFailed
Parallelize
	<Parallelize><Workers>32</Workers><Scope>MethodLevel</Scope></Parallelize>
SettingsFile
TestCleanupTimeout
TestInitializeTimeout
TestTimeout
TreatClassAndAssemblyCleanupWarningsAsErrors
TreatDiscoveryWarningsAsErrors

Here is the suggested refactored settings

{
    "platformOptions" : {
	},
	"mstest" : {
		"timeout" : {
			"assemblyInitialize" : strictly positive int,
			"assemblyCleanup" : strictly positive int,
			"classInitialize" : strictly positive int,
			"classCleanup" : strictly positive int,
			"testInitialize" : strictly positive int,
			"testCleanup" : strictly positive int,
			"test" : strictly positive int
		},
		"parallelism" : {
			"enabled": true/false,
			"workers": positive int,
			"scope": method/class,
		},
		"deployment" : {
			"enabled": true/false,
			"deployTestSourceDependencies": true/false,
			"deleteDeploymentDirectoryAfterTestRunIsComplete": true/false
		}
		... remaining settings
	}
}

I cannot find any good naming for all the props where we chose if something should be warning or error.

Note that because this is forcing the users to do some conversion, I would be in favor of changing the default for these configurations to match what we think is the good default.

Alternative Designs

None

@nohwnd
Copy link
Member

nohwnd commented Aug 16, 2024

parallelism: "workers": positive int,

This is probably an opportunity to get rid of 0 meaning unbounded, and splitting it to its own option.

execution:
MapInconclusiveToFailed
MapNotRunnableToFailed

@nohwnd
Copy link
Member

nohwnd commented Aug 20, 2024

"workers": positive int,

for this one do we want to keep the option "typed"? It could be 0-???, as well as word options like:
max
maxcpu
unbounded
allcpu

or even

halfcpu
100%
50%
30%

@Evangelink
Copy link
Member Author

I think it's fine, and a good opportunity to change to what we think is best. For the percentage, it might be tricky to respect but I am fine.

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

Successfully merging a pull request may close this issue.

3 participants