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

Extensions: Incremental additions #33563

Merged
merged 20 commits into from
Mar 17, 2020
Merged

Conversation

maryamariyan
Copy link
Member

@maryamariyan maryamariyan commented Mar 13, 2020

This PR:

  • ref/src/pkg (have no test projects)

    • Microsoft.Extensions.Caching.Abstractions
    • Microsoft.Extensions.Configuration.Abstractions
    • Microsoft.Extensions.DependencyInjection.Abstractions
    • Microsoft.Extensions.Hosting.Abstractions
    • Microsoft.Extensions.Logging.Abstractions
  • ref/src/test/pkg

    • Microsoft.Extensions.Configuration
    • Microsoft.Extensions.Configuration.Binder
    • Microsoft.Extensions.Configuration.CommandLine
    • Microsoft.Extensions.Configuration.EnvironmentVariables
    • Microsoft.Extensions.Options.ConfigurationExtensions
    • Microsoft.Extensions.Options.DataAnnotations

Next PRs:

Disabled in Directory.Build.props until pkg/test gets completed.
Could defer this to a next PR.

  • ref/src/pkg

  • test (reason for later: Extensions: Incremental additions #33563 (comment))

    • Microsoft.Extensions.DependencyInjection
    • Microsoft.Extensions.Options (Options tests needs ref to DI)
  • ref/src/pkg/test

    • Microsoft.Extensions.Caching.Memory
    • Microsoft.Extensions.Configuration.FileExtensions
    • Microsoft.Extensions.Configuration.Ini
    • Microsoft.Extensions.Configuration.Json
    • Microsoft.Extensions.Configuration.UserSecrets
    • Microsoft.Extensions.Configuration.Xml
    • Microsoft.Extensions.Hosting
    • Microsoft.Extensions.Http
    • Microsoft.Extensions.Logging.* (8x)
  • Enable tests explained in comment: Extensions: Incremental additions #33563 (comment)

  • Add solution files

- Microsoft.Extensions.Caching.Abstractions
- Microsoft.Extensions.Caching.Memory
- Microsoft.Extensions.Http

- TODO
[ ] Add Logging before Http and Caching.Memory
- Microsoft.Extensions.Caching.Abstractions
- Microsoft.Extensions.Configuration
- Microsoft.Extensions.Configuration.Abstractions
- Microsoft.Extensions.Configuration.Binder
- Microsoft.Extensions.Configuration.CommandLine
- Microsoft.Extensions.DependencyInjection
- Microsoft.Extensions.DependencyInjection.Abstractions
- Microsoft.Extensions.Logging.Abstractions
- Microsoft.Extensions.Options
+ Microsoft.Extensions.DependencyInjection
+ Microsoft.Extensions.Hosting.Abstractions

test
+ Microsoft.Extensions.Configuration.Tests
+ Microsoft.Extensions.Configuration.Binder.Tests
+ Microsoft.Extensions.Configuration.CommandLine.Tests
+ Microsoft.Extensions.Options.ConfigurationExtensions
@ericstj
Copy link
Member

ericstj commented Mar 13, 2020

src/libraries/Microsoft.Extensions.DependencyInjection/tests/Common/test/Microsoft.Extensions.Logging.Testing.Tests.csproj#L2
src/libraries/Microsoft.Extensions.DependencyInjection/tests/Common/test/Microsoft.Extensions.Logging.Testing.Tests.csproj(2,3): error MSB4019: The imported project "/Users/runner/runners/2.165.0/work/1/s/src/libraries/Microsoft.Extensions.DependencyInjection/tests/Logging.Testing/src/build/Microsoft.Extensions.Logging.Testing.props" was not found. Confirm that the expression in the Import declaration "/Users/runner/runners/2.165.0/work/1/s/src/libraries/Microsoft.Extensions.DependencyInjection/tests/Common/test/../../Logging.Testing/src/build/Microsoft.Extensions.Logging.Testing.props" is correct, and that the file exists on disk

Looks like we need to update the path in src/libraries/Microsoft.Extensions.DependencyInjection/tests/Common/test/Microsoft.Extensions.Logging.Testing.Tests.csproj

+ Microsoft.Extensions.Options.DataAnnotations
- Microsoft.Extensions.Caching.Abstractions
- Microsoft.Extensions.Configuration.Abstractions
- Microsoft.Extensions.DependencyInjection.Abstractions
- Microsoft.Extensions.Logging.Abstractions
- Microsoft.Extensions.Hosting.Abstractions
- Microsoft.Extensions.Configuration
- Microsoft.Extensions.Configuration.Binder
- Microsoft.Extensions.Configuration.CommandLine
- Microsoft.Extensions.DependencyInjection
- Microsoft.Extensions.Options
- Microsoft.Extensions.Options.ConfigurationExtensions
- Microsoft.Extensions.Options.DataAnnotations
-Microsoft.Extensions.Options
@maryamariyan
Copy link
Member Author

maryamariyan commented Mar 13, 2020

NEXT PR:

The following 6 Microsoft.Extensions test projects (under src/libraries) are not yet cleaned up from when they were brought over from Extensions repo:

Microsoft.Extensions.Configuration\tests\FunctionalTests\Microsoft.Extensions.Configuration.Functional.Tests.csproj
Microsoft.Extensions.DependencyInjection\tests\Common\test\Microsoft.Extensions.Logging.Testing.Tests.csproj
Microsoft.Extensions.Hosting\tests\FunctionalTests\Microsoft.Extensions.Hosting.Functional.Tests.csproj
Microsoft.Extensions.Hosting\tests\TestApp\Microsoft.Extensions.Hosting.TestApp.csproj
Microsoft.Extensions.Hosting\tests\UnitTests\Microsoft.Extensions.Hosting.Unit.Tests.csproj
Microsoft.Extensions.Logging\tests\Common\Microsoft.Extensions.Logging.Tests.csproj

They can't be enabled yet, until all their dependencies are completed and enabled:

  • First one needs all Configurations assemblies. (not yet)
  • Second one needs Logging, and in fact should have been moved under Logging test folder not DI
  • The remaining 4, depend on Hosting and Logging.

I will modify the Directory.Build.props file to exclude them for this PR.

@maryamariyan
Copy link
Member Author

maryamariyan commented Mar 14, 2020

NEXT PR: #33678 brings over history

  • To enable Microsoft.Extensions.DependencyInjection.Tests I need to bring over history for DI.Specification.Tests as seen in the csproj below:

https://github.com/dotnet/runtime/blob/master/src/libraries/Microsoft.Extensions.DependencyInjection/tests/Microsoft.Extensions.DependencyInjection.Tests.csproj#L10

Microsoft.Extensions.Configuration.EnvironmentVariables
DIsable Options, its test needs DI
Cleanup ProjectExclusions to skip csproj in Common folder
@maryamariyan
Copy link
Member Author

(seems unrelated but) noticed below issue in the CI: https://dev.azure.com/dnceng/public/_build/results?buildId=561783

XUnit : error : Tests failed: /root/runtime/artifacts/TestResults/Debug/AppHost.Bundle.Tests_netcoreapp5.0_x64.html [netcoreapp5.0|x64] [/root/runtime/src/installer/test/Microsoft.NET.HostModel.Tests/AppHost.Bundle.Tests/AppHost.Bundle.Tests.csproj]
##[error]XUnit(0,0): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Tests failed: /root/runtime/artifacts/TestResults/Debug/AppHost.Bundle.Tests_netcoreapp5.0_x64.html [netcoreapp5.0|x64]

- Remove DefaultReferenceExclusions and more
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants