NightOwl888 commented on PR #786: URL: https://github.com/apache/lucenenet/pull/786#issuecomment-1510583826
Thanks for the PR. I went through and completed this task, as it was clearly going to take a lot of back and forth to work out, especially being that I admittedly didn't have a complete picture of what the test was doing. Several things it does are very subtle and you have to consider the implementation of how the test framework functions in order to work them out. 1. The test has no asserts to verify the index. The test passes whether `CheckIndexes()` returns `true` or `false`. This was confusing at first, but the `TestUtil.CheckIndex()` method will throw an exception if it detects index corruption, which is the failure condition we are looking for. 2. Passing the directory path to the forked test was a bit of a challenge. Especially being that the `tmpDir` system property hadn't been implemented and the test internally calls `GetTempDirectory()` with no way to override. In fact, the test framework is still missing a few features. So, this test framework feature had to be implemented to get this test to function. 3. The test framework automatically deletes the index directory at the end of a test run (and technically, it is cleaned up prior to that at the end of the `TestNRTThreads_Mem()` method. This had me scratching my head for awhile as to how we would have any results to test on disk. The test relies on the crash to ensure the index stays persisted. This also means there will only ever by a single index (or no index) to check even though the base class test is called in a loop several times. 4. Being that the test runs inside NUnit and NUnit has safeguards in place to ensure an uncaught exception doesn't bring down the test runner, it would have been challenging to bypass this behavior. It turned out to be simpler to crash the process from the outside using [Process.Kill()](https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.kill?view=net-7.0). Unfortunately, `Process.Kill()` doesn't function to kill the current process, only another process. 5. Getting the process id of the process to kill was also challenging. `dotnet test` runs an executable named `vstest.console.exe`. This executable in turn runs `testhost.exe` which runs the tests. It is the latter we need to kill, not the process that we originally started. I went down several wrong turns before I realized that the process id could simply be logged to a file and then read back from our original test so it could start the thread to kill it after x amount of time. However, that does mean another shared path that we have to create in the original test and pass the file name to the fork. 6. I originally suggested using environment variables to configure system properties locally. However, in our CI environments we create a `lucene.testSettings.json` file rather than passing them in on the command line. This configuration file overrides any environment variables and it applies to all tests in all subdirectories where it exists. So, for the settings to override the config file, we needed to pass the settings on the command line and the syntax for doing so is [very ugly and requires escaping](https://github.com/microsoft/vstest/issues/862#issuecomment-621737720). Fortunately, because we are using `UseShellExecute = false` we don't have any inconsistency between Windows, Linux, and macOS, but it does require a different syntax to properly escape in C# in order to pass the correct escape characters into `dotnet test`. Ultimately, we got it done, though. It took a lot of research and analysis to pull off. Do note that creating the directory has been moved inside of the loop because I realized that if it were left a single shared directory all of the loops would put results into the same temp directory that is checked. And since they are checked in lexicographical order and always return on the first index checked, there is a high probability that if the first check fails most of the rest of the checks would also fail. -- 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. To unsubscribe, e-mail: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
