Improve code coverage for System.Xml namespace types

[sepidehkh]

These are some of the bigger types with low coverage in System.Xml. It would be great if we could add more tests to reach 80-90% coverage on these:

• [ ] System.Xml.XmlConvert 66.3%
• [ ] System.Xml.XmlDocument 53.8%
• [ ] System.Xml.XmlNode 65.1%
• [x] System.Xml.XmlNodeReader 29.5%
• [ ] System.Xml.XmlReader 62.9%
• [ ] System.Xml.XmlTextReader 55.1%
• [ ] System.Xml.XmlTextWriter 60.1%
• [ ] System.Xml.XmlValidatingReader 23.8%
• [ ] System.Xml.XmlWriter 32.2%
• [x] System.Xml.Resolvers.XmlPreloadedResolver 0%

Detailed coverage report: https://ci.dot.net/job/dotnet_corefx/job/master/job/code_coverage_windows/Code_Coverage_Report/

[Rajivts-zz]

Hi @sepidehMS , I'd like to participate in the issue and add relevant test cases to improve the code coverage, but I have never done open source before. I've read all the related docs and style guides, may I contribute?

[danmoseley]

@rajivts you would be welcome! Work at your own pace. @krwq owns this area now and can help with questions

[Rajivts-zz]

Thanks @danmosemsft

Hi @krwq

Before creating the test cases, I was referring the "System.Xml.XmlSchemaSet.Tests" test project for understanding the structure of existing unit tests. On inspecting the properties of the project, the target framework was found to be ".NET Portable Subset (Visual Studio 2012)". However, while searching for project templates in Visual Studio 2017 I was not able to find a project that fits this criteria, can you please provide some pointers?

Also, if its alright, can you please assign this issue to me?

[krwq]

Hello @Rajivts, If you are planning to create a new project then https://github.com/dotnet/corefx/tree/master/src/System.Private.Xml/tests/XmlDocument would be a better example. In order to create a new project please copy one of the test directories and remove/replace .cs files (and do similar changes in the csproj file).

If you just want to add tests please find a relevant project and either do everything similarly to the existing tests or simply create a new cs file in the project and just simply add Xunit's Fact or Theory (some of these tests have a little legacy behind so might not be worth following that pattern).

[krwq]

For some reason I'm not able to assign issue to you. I'll assign it to myself for now to make sure no one else picks it up. You can go ahead and work on it - thank you for offering your help!

[Rajivts-zz]

Ok, I will start working on it. Thanks for the pointers.

[danmoseley]

@krwq to assign, an admin like me or @karelz has to give collaborator rights

@Rajivts you should get an invite in mail, if you accept, we can assign issues. Note: this auto subscribes to all activity in this repo, that's a lot, you'll probably want to dial that down.

[krwq]

Thanks @danmosemsft

[Rajivts-zz]

Ok, I have accepted the invitation as per instructions.

[Rajivts-zz]

Hi @krwq

I have started with XmlNodeReader test cases and am able to see an improvement in code coverage. However, the issue description mentions the existing code coverage for System.Xml.XmlNodeReader is 29.5% but I am not able to find any existing test cases for XmlNodeReader. Am I missing something here?

[krwq]

@Rajivts - code coverage tells which code paths were hit when running tests - it does not have to be called explicitly (i.e. if other API calls XmlNodeReader APIs it counts it as well).

The other thing is I believe the numbers in this issue might not include OuterLoop tests. Please refer to https://github.com/dotnet/corefx/blob/master/Documentation/building/code-coverage.md how to run code coverage. You might also add /p:OuterLoop=true to make sure outerloop tests run as well.

Please also see our official code coverage report: https://ci.dot.net/job/dotnet_corefx/job/master/job/code_coverage_windows/Code_Coverage_Report

[Rajivts-zz]

Hi @krwq

I can see the 29.5% coverage now, and for running my own tests I am using the commands mentioned in the code-coverage document above. However, it would make the job much easier if I could run the tests within Visual Studio, which I am currently struggling with. I am getting the following message on trying to run the tests in Visual Studio: "No test is available. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again." Are there any per-requisites that I am unaware of?

[krwq]

@Rajivts:

• make sure you use VS 2017 (community version is free)
• make full build as you usually do in corefx (i.e. build.cmd)
• open related sln file
• right click on test project and "Set us startup project" (or something like that)
• try running

if you want to edit the command line to run coverage go to project options and in the Debug tab edit command line as desired

[Rajivts-zz]

Hi @krwq

I have completed with the first round of test case creation for XmlNodeReader and achieved a code coverage of 86.6%. Since I am unable to create a remote branch on corefx repo, how should I create a PR to check-in my changes?

[danmoseley]

@Rajivts you should "fork" the repo, and push your change to a branch in your fork. Then you can create a PR from your fork to this master repo.

There's a bunch of info here: https://help.github.com/categories/collaborating-with-issues-and-pull-requests/

in particular

https://help.github.com/articles/configuring-a-remote-for-a-fork/ https://help.github.com/articles/merging-an-upstream-repository-into-your-fork/ https://help.github.com/articles/creating-a-pull-request-from-a-fork/

[Rajivts-zz]

Created a pull request with the associated changes: dotnet/corefx#31129 . Please review.

[Rajivts-zz]

@danmosemsft Updated the PR to incorporate the review comments. Please let me know if any more changes are required.

[Rajivts-zz]

Hi @krwq

In the unit test project that I created, the properties and targets file are being referenced from the src directory as is the case for XmlSchema unit test projects. However, the gated build works fine for XmlSchema but fails for my unit test project. Can you please let me know of some other way I can specify the targets and properties file for my project?

[krwq]

@Rajivts I think you need to copy Configurations.props as well

[Rajivts-zz]

@krwq , I wish to write test cases for "System.Xml.Resolvers.XmlPreloadedResolver" next. Should I go ahead?

[danmoseley]

@rajivts as far as I know nobody else is currently working in XML tests so there is no need to ask if you wish to work on them. Your efforts are welcome.

[krwq]

@Rajivts yes, go ahead :-) Thank you for offering your help!

[Rajivts-zz]

@danmosemsft @krwq Created PR: dotnet/corefx#31273 with unit test cases for XmlPreloadedResolver. The effective code coverage is 96%

[Rajivts-zz]

@krwq Updated the PR with changes adhering to your recommendations. Please review.

[Rajivts-zz]

@krwq Created PR dotnet/corefx#31501 containing test cases for System.Xml.XmlTextReader. The resultant code coverage is ~95%. The only methods not covered are 2 protected constructors, which were not referenced from anywhere. I am assuming its not dead code and is actually used from some class.

[danmoseley]

To exercise protected constructors define a sub class in your test and call them from within it