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 information about which assembly failed to discover test into #299

Merged
merged 5 commits into from
Jan 5, 2018

Conversation

kant2002
Copy link
Contributor

Having indicate location of the assembly from which discovery happens allow developer reason about possible failures, and manually troubleshoot his configuration issues which cause asembly to fail

This allow simplify resolution of the issues like #295 and similar where it is not clear what exactly happens. For example when you run tests from multiple assemblies, like in VSTS CI build

Copy link
Contributor

@AbhitejJohn AbhitejJohn left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @kant2002 . Looks good overall . Can you add/modify tests as well please?
Also please run build.cmd -uxlf so the string addition is localizable. You would have to commit the xlf file changes post running the script.

var winrtFailureMessage = string.Format(
CultureInfo.CurrentCulture,
Resource.TestAssembly_WinRTAssemblyDiscoveryFailure,
fullFilePath,
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be assemblyFullName.

@@ -299,4 +299,10 @@
<data name="DiscoveryWarning" xml:space="preserve">
<value>[MSTest][Discovery][{0}] {1}</value>
</data>
<data name="TestAssembly_AssemblyDiscoveryFailure" xml:space="preserve">
<value>Failed to discover tests from assembly located at {0}. Reason:{1}</value>
Copy link
Contributor

@AbhitejJohn AbhitejJohn Oct 25, 2017

Choose a reason for hiding this comment

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

Suggestion: Can we only have one string with a

Failed to discover tests from assembly {0}. Reason:{1}

CultureInfo.CurrentCulture,
Resource.TestAssembly_AssemblyDiscoveryFailure,
fullFilePath,
ex.Message);
warnings.Add(ex.Message);
Copy link
Member

Choose a reason for hiding this comment

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

This should be warnings.Add(message)

@AbhitejJohn
Copy link
Contributor

@kant2002 : There appears to be a unit test failing - GetTestsShouldReturnNullIfSourceFileDoesNotExistInContext from here. Can you take a look please?
The validation scripts seem to be having an issue which needs fixing.

@kant2002
Copy link
Contributor Author

Ooh, I forgot to run tests, since was sidetracked with build issues. Will take a look.

@kant2002
Copy link
Contributor Author

@AbhitejJohn I actually blocked with #300 without ability to do clean build, so when I build solution the failed test does not run, since assembly where it is probably missing. Could you take a look what could I install to make build works.

@jayaranigarg
Copy link
Member

@kant2002 : We have fixed the nuget package restore issue here. You can now go ahead and update this PR.

@jayaranigarg
Copy link
Member

@kant2002 : Following up on this. Can you please do the needful to complete this PR?

@kant2002
Copy link
Contributor Author

kant2002 commented Jan 4, 2018

@jayaranigarg thanks for your patience, I have some issues with my VS setup after 15.4 update probably which prevent me from do clean build.

D:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\Microsoft\WindowsXaml\v15.0\8.2\Microsoft.Windows.UI.Xaml.Common.targets(676,5): warning : Could not find target platform winmds for the SDK specified by [Windows, 10.0, UAP, 10.0.10240.0, 10.0.14393.0] [xxxx\testfx\src\TestFramework\Extension.UWP\Extension.UWP.csproj]

I try to reinstall VS now and check patch again. I will report back when I finish with VS install

Having indicate location of the assembly from which discovery happens allow developer reason about possible failures, and manually troubleshoot his configuration issues which cause asembly to fail
* Single messages for assembly loading failure
* Display assembly name for WinRT assembly loading
@kant2002
Copy link
Contributor Author

kant2002 commented Jan 4, 2018

@jayaranigarg or @AbhitejJohn please take a look

@jayaranigarg
Copy link
Member

@kant2002 : Thank you for completing this. Looks good to me. Will push this in.

@jayaranigarg jayaranigarg merged commit f665460 into microsoft:master Jan 5, 2018
@kant2002 kant2002 deleted the diagnostic-improvements branch January 5, 2018 09:21
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.

4 participants