System.Configuration.ConfigurationManager needs more test coverage (51.8%)

[JeremyKuhne]

#19296 ported the code for System.Configuration to facilitate porting existing .NET code. Test coverage is relatively sparse as this effort was meant to be demand driven. Adding additional tests (unit and scenario) is a good opportunity to get one's feet wet in CoreFX.

ConfigurationPermission is the only class that wasn't ported (Code Access Security isn't supported in CoreFX). The only known non-working class is RsaProtectedConfigurationProvider which is dependent on System.Security.Cryptography.Xml working (#14950).

[danmoseley]

Line coverage is 42%
https://ci.dot.net/job/dotnet_corefx/job/master/job/code_coverage_windows/Code_Coverage_Report/

[dhoehna]

I don't know the process of taking an issue, but I volunteer to increase the test coverage.

[JeremyKuhne]

@karelz can you assign this to @dhoehna?

[karelz]

@dhoehna collaborator invite sent. Please ping us when you accept and we will assign it to you. Assigning to @JeremyKuhne in the meantime ;-)

[karelz]

@JeremyKuhne ca you please add issue type? (I assume test enhancement)

[dhoehna]

@karelz I have accepted and enabled two-factor authentication.

[danmoseley]

@dhoehna this may help https://github.com/dotnet/corefx/blob/master/Documentation/building/code-coverage.md

[dhoehna]

@karelz Now I have accepted the invitation.

[karelz]

Reassigned to you @dhoehna, thanks for your contribution in the space!

[dhoehna]

@karelz Thank you. What is the priority for this issue and what does the priority mean? Also, is there a deadline when this issue has to be resolved?

[karelz]

Milestone is Future, which means we (.NET team) don't plan to invest into it during 2.0 timeframe. Other than that, no priority or deadline.
It's nice to update the issue every week or two to communicate progress is being made - if there's no progress, others might want to pick it up.

[dhoehna]

Okay. Thank you.

[dhoehna]

Another question. I only see a folder called System.Configuration.ConfigurationManager. Is this the folder that needs better test coverage, or am I looking in the wrong spot?

[FireAlkazar]

@dhoehna can I join this issue? It looks like a great start point to dive into corefx contribution.

[dhoehna]

@FireAlkazar I don't mind. Once you are added, we can divvy up the work. @karelz Is it possible to add another contributor?

[JeremyKuhne]

@dhoehna System.Configuration.ConfigurationManager is where all of the code is. We couldn't use the same exact assembly name as an existing .NET assembly, this was our best compromise. :)

[karelz]

@FireAlkazar added, please coordinate the work and/or tell us (me and @JeremyKuhne) if you need help or guidance.

[dhoehna]

@FireAlkazar How to you want to divvy it up? I'm thinking of alphabetically.

[FireAlkazar]

@dhoehna I want to try at first to cover ConnectionStrings property of Configuration. Write some simple tests and check if they are ok. And move forward after getting some feedback.

[dhoehna]

Okay. I'll start from the beginning than.

[dhoehna]

Am I doing something wrong? I can't build System.Configuration.ConfigurationManager. I get errors like "Missing compiler required member 'System.ParamArrayAttribute..ctor'" and "Predefined type System.Enum is not defined or imported" and "Predefined type System.Int32 is not defined or imported"

Here is what I have done

1. Fork the repo to my desktop
2. git pull on the corefx repo
3. Open Visual Studio
4. Run Build->Rebuild solution.

I've done 4 years of development on VS and I've done VS development on my current computer, so I don't know what is wrong.

Also, I am using Visual Studio 2015.

EDIT: I'm assuming the project builds successfully from master.
I don't want to add any references in fear of messing something up..

EDIT2: +@karelz

[JeremyKuhne]

@dhoehna you need to run the build.cmd from the root once. In order to run the tests you'll need to run the build-tests.cmd once. After that you'll be able to build with the projects directly either through the command line (e.g. msbuild.exe) or in VS.

To run all of the tests from VS you need to set the test project as the startup project. We don't have things set up to use the test explorer yet.

[dhoehna]

Oh. I thought I did run build.cmd. I am running it now. I'll let you know if I have any problems.

[dhoehna]

@JeremyKuhne I am still having issues.

Build.cmd ran to completion.
build-tests.cmd gave me yellow and red text at the end, saying some tests failed.

I opened visual studio and got the errors as described above but I was able to build all projects in System.Configuration.ConfigurationManager.

I do get an error when I try to run the test class though. I pasted the image from my command prompt.

[JeremyKuhne]

It would be better if you pasted the output rather than an image. (Just block it in ```.)

What solution configuration is active?

You should try building the test project from the command line. Run the developer command prompt and in the test directory run corefx\src\System.IO.Ports\tests>msbuild System.IO.Ports.Tests.csproj /t:BuildAndTest.

If things are messed up it is also worth trying /t:RebuildAndTest.

If you can run tests fine from the command line that would mean VS is in the way. Possibly the configuration isn't set right.

The spew from my command line run is:

  Command(s):
  call P:\repos\corefx\Tools\testdotnetcli\dotnet.exe xunit.console.netcore.exe System.IO.Ports.Tests.dll  -xml testRes
  ults.xml -notrait Benchmark=true -notrait category=nonnetcoreapp1.1tests  -notrait category=OuterLoop -notrait catego
  ry=failing -notrait category=nonwindowstests
  xUnit.net console test runner (64-bit .NET Core)
  Copyright (C) 2014 Outercurve Foundation.

  Discovering: System.IO.Ports.Tests
  Discovered:  System.IO.Ports.Tests
  Starting:    System.IO.Ports.Tests
     Close.OpenFillBuffersClose [SKIP]
        Condition(s) not met: \"HasNullModem\"
     Close.CloseWithoutOpen [SKIP]
        Condition(s) not met: \"HasOneSerialPort\"
     Close.OpenClose [SKIP]
        Condition(s) not met: \"HasOneSerialPort\"
     Close.OpenCloseNewInstanceOpen [SKIP]
        Condition(s) not met: \"HasOneSerialPort\"
  Finished:    System.IO.Ports.Tests

  === TEST EXECUTION SUMMARY ===
     System.IO.Ports.Tests  Total: 17, Errors: 0, Failed: 0, Skipped: 4, Time: 0.325s
  Finished running tests.  End time=15:27:35.00, Exit code = 0

It is also possible you're in a bad state in general (with all of the constant build changes we're making this can happen). Run clean.cmd in the root, then build.cmd and build-tests.cmd again (I just had to to get building again in VS.).

Here is my console when running in VS:

xUnit.net console test runner (64-bit .NET Core)
Copyright (C) 2014 Outercurve Foundation.

Discovering: System.IO.Ports.Tests
Discovered:  System.IO.Ports.Tests
Starting:    System.IO.Ports.Tests
   Close.OpenFillBuffersClose [SKIP]
      Condition(s) not met: \"HasNullModem\"
   Close.CloseWithoutOpen [SKIP]
      Condition(s) not met: \"HasOneSerialPort\"
   Close.OpenClose [SKIP]
      Condition(s) not met: \"HasOneSerialPort\"
   Close.OpenCloseNewInstanceOpen [SKIP]
      Condition(s) not met: \"HasOneSerialPort\"
Finished:    System.IO.Ports.Tests

=== TEST EXECUTION SUMMARY ===
   System.IO.Ports.Tests  Total: 17, Errors: 0, Failed: 0, Skipped: 4, Time: 3.742s

Press enter key to continue...

[Jlalond]

I understand what you're saying, but we're checking for a type, and said type is a concrete type, and we have an object of that type, and I'm confused why it's still returning null

[Jlalond]

Hey all, an update. I am still working on this, but took a small Hiatus (I moved and everything has been crazy!) Just wanted to give everyone an update. I hope to send our a PR next week

[stephentoub]

Thanks for the update.

[Jlalond]

@stephentoub Question for you. Trying to Test AppSettingsReader and some other of the bigger classes that read from the config xml's. I'm struggling to figure out how to create a test app config. I created an app config in the test assembly, but A. it doesn't read from it's appsettings, and B that doesn't feel like a good way to 'mock' a test app config at all. Any Ideas?

[danmoseley]

@JeremyKuhne

[JeremyKuhne]

@Jlalond here is the pattern I was using. https://github.com/dotnet/corefx/blob/master/src/System.Configuration.ConfigurationManager/tests/System/Configuration/SmokeTest.cs

[Jlalond]

Thanks @JeremyKuhne

[Jlalond]

@JeremyKuhne So still some questions. App settings reader's constructor references ConfigurationManager.AppSettings directly, how should I create a temporary config for ConfigurationManager to reference. I was hoping incorrectly that the using block would temporarily change the underlying ConfigurationManager app config, but I was wrong. So I'm still stuck.

[JeremyKuhne]

It is no different than hitting the Configuration.AppSettings property- ConfigurationManager simply has a specific Configuration object that it creates based off of it's own path heuristics. You should be able to test everything in AppSettings off the Configuration object explicitly loaded as per the example I gave you.

[eriawan]

@JeremyKuhne

Since the PR to fix this is already merged, shall we close this or do we need to increase more coverage?

[JeremyKuhne]

shall we close this or do we need to increase more coverage?

There is still more room for improvement. dotnet/corefx#29896 is a step forward, but we have more steps to make. :)

[Jlalond]

Hey @JeremyKuhne might want to unassign me to this. Had to change my past time focuses a little bit. Had the luck of getting hit by a car biking, and lately I've been dedicating more time to getting fit again after my hiatus. Plus it'll give someone entry to the project!

[JeremyKuhne]

@Jlalond no problem- you're always welcome to come back to this or any other issue

[Rutix]

Hi @JeremyKuhne I want to give a shot to up the code coverage for this issue :). Any pointers which parts are the big wins ? Or should I just look at https://ci.dot.net/job/dotnet_corefx/job/master/job/code_coverage_windows/Code_Coverage_Report/ and pick the ones with the biggest blocks uncovered?

[JeremyKuhne]

@Rutix Great! Pick whatever seems interesting to you off of that list. One other suggestion is to go through the docs and try to make sure we have tests covering the described scenarios there.

[Rutix]

Since it would be handy to see what impact my tests have on the code coverage I tried to get coverage working locally. When I do dotnet msbuild /t:buildandtest /p:Coverage=true I get the following and nothing in the index.htm

Rutger@FATE  F\..\corefx\..\..\tests   master ≣  dotnet msbuild /t:buildandtest /p:Coverage=true
Microsoft (R) Build Engine version 15.8.166+gd4e8d81a88 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  RemoteExecutorConsoleApp -> F:\projects\corefx\bin\AnyOS.AnyCPU.Debug\RemoteExecutorConsoleApp\netstandard\RemoteExecutorConsoleApp.exe
  System.Configuration.ConfigurationManager.Tests -> F:\projects\corefx\bin\AnyOS.AnyCPU.Debug\System.Configuration.ConfigurationManager.Tests\netstandard\System.Configuration.ConfigurationManager.Tests.dll
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning : SymUnmanagedWriterException: Specified method is not supported. [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :    at Microsoft.DiaSymReader.SymUnmanagedWriterFactory.CreateWriter(ISymWriterMetadataProvider metadataProvider, SymUnmanagedWriterCreationOptions options) in /_/src/Microsoft.DiaSymReader/Writer/SymUnmanagedWriterFactory.cs:line 86 [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :    at Microsoft.DiaSymReader.Tools.PdbConverter.ConvertPortableToWindows(PEReader peReader, MetadataReader pdbReader, Stream targetPdbStream, PortablePdbConversionOptions options) in /_/src/Microsoft.DiaSymReader.Converter/PdbConverter.cs:line 169 [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :    at Microsoft.DiaSymReader.Tools.PdbConverter.ConvertPortableToWindows(PEReader peReader, Stream sourcePdbStream, Stream targetPdbStream, PortablePdbConversionOptions options) in /_/src/Microsoft.DiaSymReader.Converter/PdbConverter.cs:line 143 [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :    at Microsoft.DotNet.Build.Tasks.ConvertPortablePdbsToWindowsPdbs.ConvertPortableToWindows(ITaskItem file, PdbConverter converter, PortablePdbConversionOptions parsedConversionOptions) [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :    at Microsoft.DotNet.Build.Tasks.ConvertPortablePdbsToWindowsPdbs.Execute() [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning : NotSupportedException: Specified method is not supported. [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :    at Microsoft.DiaSymReader.SymUnmanagedWriterFactory.CreateWriter(ISymWriterMetadataProvider metadataProvider, SymUnmanagedWriterCreationOptions options) in /_/src/Microsoft.DiaSymReader/Writer/SymUnmanagedWriterFactory.cs:line 81 [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
F:\projects\corefx\bin/testhost/netcoreapp-Windows_NT-Debug-x64/shared/Microsoft.NETCore.App/10.10.10/System.Configuration.ConfigurationManager.dll : warning :  [F:\projects\corefx\src\System.Configuration.ConfigurationManager\tests\System.Configuration.ConfigurationManager.Tests.csproj]
  Replacing Portable PDB with Windows Pdb in place!
  Using F:\projects\corefx\bin\testhost\netcoreapp-Windows_NT-Debug-x64\ as the test runtime folder.
  Executing in F:\Projects\corefx\bin\tests\System.Configuration.ConfigurationManager.Tests\netcoreapp-Windows_NT-Debug-x64\
  ----- start 22:11:40.03 ===============  To repro directly: =====================================================
  pushd F:\Projects\corefx\bin\tests\System.Configuration.ConfigurationManager.Tests\netcoreapp-Windows_NT-Debug-x64\
  call F:\projects\corefx\packages/OpenCover\4.6.519\tools\OpenCover.Console.exe -oldStyle -filter:+[System.Configuration.ConfigurationManager]* -excludebyfile:*\Common\src\System\SR.* -nodefaultfilters -excludebyattribute:*.ExcludeFromCodeCoverage* -skipautoprops -hideskipped:All -threshold:1 -returntargetcode -register:user -target:F:\projects\corefx\bin\testhost\netcoreapp-Windows_NT-Debug-x64\\dotnet.exe -output:F:\projects\corefx\bin/tests/coverage\System.Configuration.ConfigurationManager.Tests.coverage.xml -targetargs:xunit.console.dll System.Configuration.ConfigurationManager.Tests.dll  -xml testResults.xml -notrait category=nonnetcoreapptests -notrait category=nonwindowstests  -notrait category=OuterLoop -notrait category=failing  -notrait Benchmark=true
  popd
  ===========================================================================================================
  Executing: F:\projects\corefx\bin\testhost\netcoreapp-Windows_NT-Debug-x64\dotnet.exe
  xUnit.net Console Runner v2.4.1-pre.build.4059 (64-bit .NET Core 4.6.26827.04)
    Discovering: System.Configuration.ConfigurationManager.Tests
    Discovered:  System.Configuration.ConfigurationManager.Tests
    Starting:    System.Configuration.ConfigurationManager.Tests
    Finished:    System.Configuration.ConfigurationManager.Tests
  === TEST EXECUTION SUMMARY ===
     System.Configuration.ConfigurationManager.Tests  Total: 616, Errors: 0, Failed: 0, Skipped: 0, Time: 1.882s
  Committing...
  No results, this could be for a number of reasons. The most common reasons are:
      1) missing PDBs for the assemblies that match the filter please review the
      output file and refer to the Usage guide (Usage.rtf) about filters.
      2) the profiler may not be registered correctly, please refer to the Usage
      guide and the -register switch.
  ----- end 22:11:51.09 ----- exit code 0 ----------------------------------------------------------
  Loading report 'F:\projects\corefx\bin\tests\coverage\System.Configuration.ConfigurationManager.Tests.coverage.xml'
   Preprocessing report
   Initiating parser for OpenCover
  Initializing report builders for report types: Html, Badges
  Analyzing 0 classes
   Creating summary
  Report generation took 0.3 seconds

Any ideas of whats going wrong?

[vnwonah]

Still figuring out what is wrong. I am making progress. Hopefully my efforts will help someone else down the road.

@FireAlkazar Sorry I'm not helping. It seems like both my computers have something wrong with them and won't let me run corefx code.

Just letting you know that your efforts is helping me build this now. :) I'm following the conversation and fixing issues I encounter.

[danmoseley]

Hi @FireAlkazar @vnwonah are you folks still working on this one? Also do either of you know the current coverage %?

[danmoseley]

I'm going to close this as it seems like a low priority given we plan to make no changes to this library and recommend against its use in new code.