lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [lucenenet] NightOwl888 commented on pull request #287: Implemenation of "System Properties" in .NET (addresses #254)
Date Wed, 27 May 2020 13:32:53 GMT

NightOwl888 commented on pull request #287:
URL: https://github.com/apache/lucenenet/pull/287#issuecomment-634664029


   These changes have now been merged to master in commit 4ef61cba234ab87f9751c07f278a833d311d2c7a.
   
   ## Dependency Injection Proof-of-Concept
   
   Note that I fixed the integration point for adding dependencies to the test framework and
added a [Lucene.Net.Tests.TestFramework.DependencyInjection](https://github.com/apache/lucenenet/tree/4ef61cba234ab87f9751c07f278a833d311d2c7a/src/Lucene.Net.Tests.TestFramework.DependencyInjection)
project as proof-of-concept for supplying dependencies via `Microsoft.Extensions.DependencyInjection`.
The project contains both an `IConfigurationFactory` and a `ICodecFactory` implementation
to fill the gap and let the DI container manage the lifetime of the services.
   
   To fit more in line with the Microsoft convention, the `LuceneTestFrameworkInitializer`
implementations were named [`Startup.cs`](https://github.com/apache/lucenenet/blob/4ef61cba234ab87f9751c07f278a833d311d2c7a/src/Lucene.Net.Tests.TestFramework.DependencyInjection/Startup.cs),
as they technically serve the same purpose as the application's composition root.
   
   > **NOTE:** For a custom `LuceneTestFrameworkInitializer` subclass to function correctly,
[NUnit requires it to be placed outside of any namespace](https://stackoverflow.com/a/3188421).
This ensures it will run before and after any tests in any namespace. Each test assembly can
have exactly 1 `LuceneTestFrameworkInitializer` subclass (by convention, named `Startup.cs`).
   
   ## Toxic Method Calls in Tests
   
   Calling `ConfigurationSettings.SetConfigurationFactory()` from `LuceneTestCase.BeforeClass()`,
`LuceneTestCase.SetUp()` or within tests is not a supported scenario and will lead to undefined
behavior.
   
   The implementation of `IConfiguartionFactory` has been changed to return `IConfiguration`
rather than `IConfigurationRoot` for the following reasons:
   
   1. To hide the toxic `Reload()` method which is not supported in test scenarios and will
lead to undefined behavior.
   2. To fit more in line with the default ASP.NET Core template, which uses `IConfiguration`
in its `Startup` class.
   
   The only scenario where `Reload()` makes sense is to test the implementation of it in our
custom `IConfiguartionBuilder` implementation. This feature may be used in a real-world application,
but has no valid purpose in the test framework for end users.
   
   The name of the method in `IConfigurationFactory` was also changed to `GetConfiguration()`
to match the convention of the codec factories.
   
   ## Test Configuration Files
   
   The tests were changed to embed the configuration files and output them to the user's temp
directory and to use mock names so they don't interfere with the test framework itself.
   
   The [`src/lucene.testSettings.json`](https://github.com/bongohrtech/lucenenet/blob/feature/LUCENENET-638/src/lucene.testSettings.json)
file was also removed (also to keep it from interfering with the tests) and only exists in
this branch. We may need a cut down version of it for documentation purposes, but most of
the settings there only apply to Java Lucene.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message