Fix desktop test searching #23793
Fix desktop test searching #23793
Conversation
build/scripts/tests.sh
Outdated
| @@ -20,10 +20,15 @@ xunit_console_version="$(get_package_version dotnet-xunit)" | |||
|
|
|||
| if [[ "${runtime}" == "dotnet" ]]; then | |||
| target_framework=netcoreapp2.0 | |||
| dir_list="${unittest_dir}"/*/netcoreapp2.0/*.UnitTests.dll | |||
There was a problem hiding this comment.
I would probably prefer:
dir_list=( "${unittest_dir}"/*/netcoreapp2.0/*.UnitTests.dll )
and for file_name in "${dir_list[@]}" (and removal of dir_list="${unittest_dll_list[@]}").
The way you have it now is susceptible to spaces in paths, I think - or at least really confusing (I don't think variable assignments expand globs, so the expansion is happening at for-loop-time - while being a statically known list for mono. Using an array (my preference above) makes the expansion happen at declaration-time)
There was a problem hiding this comment.
Actually, how does this work in the first place?? My testing:
khyperia@carbon ~ i % foo=*
khyperia@carbon ~ i % echo $foo
*
khyperia@carbon ~ i % for x in $foo; do; echo $x; done
*
khyperia@carbon ~ i % bar=( * )
khyperia@carbon ~ i % echo $bar
file1 file2 file3
khyperia@carbon ~ i % for x in "${bar[@]}"; do; echo $x; done
file1
file2
file3
khyperia@carbon ~ i %
There was a problem hiding this comment.
The variable should also probably be called file_list as well
build/scripts/tests.sh
Outdated
| target_framework=net461 | ||
| unittest_dll_list=( | ||
| "${unittest_dir}/CSharpCompilerSymbolTest/net461/Roslyn.Compilers.CSharp.Symbol.UnitTests.dll" | ||
| "${unittest_dir}/CSharpCompilerSyntaxTest/net461/Roslyn.Compilers.CSharp.Syntax.UnitTests.dll" |
There was a problem hiding this comment.
I'm guessing this is the static list of what dlls were actually running (positive list instead of negative list). Good to keep in mind for the future.
Edit: never mind, I'm dumb, didn't read the description :)
Not all of our desktop tests have a target framework directory in their paths. Only the ones which also multi-target do. For the time being changing the set of DLLs that we run on Mono to be a hard coded list as it's only two. Will expand out as we increase our scope.
build/scripts/tests.sh
Outdated
| xunit_console="${nuget_dir}"/dotnet-xunit/"${xunit_console_version}"/tools/${target_framework}/xunit.console.dll | ||
| elif [[ "${runtime}" == "mono" ]]; then | ||
| source ${root_path}/build/scripts/obtain_mono.sh | ||
| target_framework=net461 | ||
| unittest_dll_list=( |
There was a problem hiding this comment.
Blarg ... so basic of a mistake ...
Not all of our desktop tests have a target framework directory in their
paths. Only the ones which also multi-target do. For the time being
changing the set of DLLs that we run on Mono to be a hard coded list as
it's only two. Will expand out as we increase our scope.