You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2021/02/18 22:08:35 UTC
[GitHub] [lucenenet] rclabo opened a new pull request #425: Reviewed All codec tests. No real issues found. Did some cleanup.
rclabo opened a new pull request #425:
URL: https://github.com/apache/lucenenet/pull/425
Added comments and removed unneeded commented out using statements. This contribution may not look like much, but it's the result of 10 hours of work. I not only reviewed all Test Classes in Lucene.Net.Tests.Codecs namespace but also reviewed all code for the base classes they use which often lived in other namespaces. All code review was done side by side against the java implementation.
----------------------------------------------------------------
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
[GitHub] [lucenenet] NightOwl888 merged pull request #425: Reviewed All codec tests. No real issues found. Did some cleanup.
Posted by GitBox <gi...@apache.org>.
NightOwl888 merged pull request #425:
URL: https://github.com/apache/lucenenet/pull/425
----------------------------------------------------------------
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
[GitHub] [lucenenet] NightOwl888 commented on a change in pull request #425: Reviewed All codec tests. No real issues found. Did some cleanup.
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on a change in pull request #425:
URL: https://github.com/apache/lucenenet/pull/425#discussion_r579170086
##########
File path: src/Lucene.Net.Tests.Codecs/SimpleText/TestSimpleTextStoredFieldsFormat.cs
##########
@@ -29,6 +29,10 @@ protected override Codec GetCodec()
return new SimpleTextCodec();
}
+
Review comment:
This comment had been left in the project by mistake, so please remove it. Early versions of NUnit on with VS2017 grouped tests in an abstract base class in a separate "unknown" project instead of putting them in the project where they exist. This made it cumbersome to select all of the tests in a specific project to run. However, NUnit addressed that issue and most of these unnecessary overrides to prevent that issue were taken out.
That said, this override is necessary because only this codec should be marked with the `[Deadlock]` attribute, as it is the only one that is known to have contention issues. However, it should be removed again once we track down the contention issues that are causing it to slow down.
The overrides in [TestFSTOrdPostingsFormat](https://github.com/apache/lucenenet/blob/6605cb9d8b536b7d695adcb1d84e78a417862f51/src/Lucene.Net.Tests.Codecs/Memory/TestFSTOrdPostingsFormat.cs) can be removed, and please do so.
> NOTE: There is still an issue with the current version of NUnit where the reporting of an error shows the abstract base class instead of the subclass in CI, which makes the test that caused the error tough to pin down sometimes. Ideally we should find a solution that doesn't require us to override every method, though.
----------------------------------------------------------------
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