CSHARP-4169: Move unified spec tests to root folder. #890
Conversation
|
NOTE: Although "Files changed" says 1,719, the vast majority are renames (AKA moves from one directory to another). Actual real changes were only in 4 |
| return base.ShouldReadJsonDocument(path) && | ||
| !MonitoringPrefixes.Any(prefix => path.StartsWith(prefix)) && | ||
| !path.Contains(loadBalancerTestDirectory); // load balancer support not yet implemented | ||
| !path.StartsWith(IntegrationTestPrefix); |
There was a problem hiding this comment.
loadBalancerTestDirectory evaluated to :load-balanced: but the test paths were .load_balanced.. Thus we've been running the load balancer spec tests successfully for months now since the code to ignore them was incorrect. 🤷♂️ I decided to simply remove the condition since the load balancer tests are running successfully.
| @@ -606,6 +606,9 @@ private class TestCaseFactory : JsonDrivenTestCaseFactory | |||
| "MongoDB.Driver.Core.Tests.Specifications.server_discovery_and_monitoring.tests.legacy_hello.monitoring." | |||
| }; | |||
|
|
|||
| // Integration tests are run by ServerDiscoveryAndMonitoringIntegrationTestRunner in MongoDB.Driver.Tests | |||
There was a problem hiding this comment.
did we discuss avoid placing unrelated to the current assembly tests?
There was a problem hiding this comment.
I guess it's done for simplicity, but then it also means that the initial goal is not reached. Maybe it's not a big deal though
There was a problem hiding this comment.
I only included top-level specs necessary for each csproj. But there were a few examples - like SDAM and Read/Write Concern - where we had some subdirectories of tests that were only used in Core while others were used in Driver. Rather than getting really specific with <EmbeddedResource/> directives, I decided it was easier to omit certain test subdirectories via PathPrefix shenanigans as we do elsewhere.
| @@ -44,7 +44,7 @@ public void Run(JsonDrivenTestCase testCase) | |||
| public class TestCaseFactory : JsonDrivenTestCaseFactory | |||
| { | |||
| // protected properties | |||
| protected override string PathPrefix => "MongoDB.Driver.Tests.Specifications.read_write_concern.tests."; | |||
| protected override string PathPrefix => "MongoDB.Driver.Tests.Specifications.read_write_concern.tests.operation."; | |||
There was a problem hiding this comment.
operation? Where it's came from?
There was a problem hiding this comment.
read-write-concern contains connection-string, document, and operation subdirectories. The first two are handled by Core.Tests whereas the last one is handled by Driver.Tests. Rather than including all and then omitting connection-string and document it was easier to only run the spec tests in the operation subdirectory.
|
Note regarding:
My interpretation is slightly different. LinkBase PRIMARILY affects the path of the embedded resources. It results in the path of the embedded resources being the same as they were before the JSON files were moved (in the file system), so that the test runners can continue to find them without requiring changes to the test runner. What an IDE does with this information varies from IDE to IDE. In Visual Studio 2022 it appears to continue to display the JSON test files in the same locations as before (deep inside the individual test projects). In Rider it displays them in the root of the Solution Explorer (according to James). |
To clarify, Rider also displays the JSON files in their original locations just as VS2022 does. |
| @@ -103,4 +103,10 @@ | |||
| <Folder Include="Specifications\transactions-convenient-api\tests\" /> | |||
| </ItemGroup> | |||
|
|
|||
| <ItemGroup> | |||
| <Compile Include="..\..\specifications\atlas-data-lake-testing\AtlasDataLakeTestRunner.cs"> | |||
There was a problem hiding this comment.
can you please clarify why did you change it?
There was a problem hiding this comment.
I diff'd our $/specifications directory with the mongodb/specifications repo and noticed two discrepancies:
- In ours, the spec was called
atlas-data-lakebut in the spec repo it is calledatlas-data-lake-testing. - In ours, the spec was called
bsonbut in the spec repo it is calledbson-decimal128.
I renamed our two directories to match the directory names in the spec repo. None of the tests were modified or updated. The only change was in the runners to account for the new directory name.
There was a problem hiding this comment.
Oh! Good catch. That cs file should not have been moved, only the JSON files. Looks like I fat-fingered something. I'll fix it.
JamesKovacs
force-pushed
the
csharp4169
branch
from
6282b56 to
79a677d
Compare
|
NOTE: Force-pushed after rebase and squash. Verified that total tests before and after the re-organization are the same. |
Relocated all spec tests to
$/specificationsto match the layout ofmongodb/specifications. AddedLinkedBaseto our existingEmbeddedResourcedirectives in thecsprojfiles to keep the Solution Explorer layout the same. I made some minor updates to test runners to ensure the correct unified tests were sourced.