NightOwl888 commented on code in PR #1051: URL: https://github.com/apache/lucenenet/pull/1051#discussion_r1860754285
########## src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs: ########## @@ -67,7 +67,7 @@ namespace Lucene.Net.Util /// /// <para> /// The preferred way to specify class (suite-level) setup/cleanup is to use - /// static methods annotated with <see cref="OneTimeSetUp"/> and <see cref="OneTimeTearDown"/>. Any + /// static methods annotated with <see cref="NUnit.Framework.OneTimeSetUpAttribute"/> and <see cref="NUnit.Framework.OneTimeTearDownAttribute"/>. Any Review Comment: This documentation is incorrect for Lucene.NET because NUnit calls static methods annotated with the attributes in the wrong order. It should read: ``` The preferred way to specify class (suite-level) setup/cleanup is to override <see cref="OneTimeSetUp"/> and <see cref="OneTimeTearDown"/>. Be sure to call <c>base.OneTimeSetUp()</c> BEFORE you initialize your class and call <c>base.OneTimeTearDown()</c> AFTER you clean up your class. NUnit will find the <see cref="NUnit.Framework.OneTimeSetUpAttribute"/> and <see cref="NUnit.Framework.OneTimeTearDownAttribute"/> of the base class, so using them on the <see cref="OneTimeSetUp"/> and <see cref="OneTimeTearDown"/> method overrides is not strictly required. Any ``` ########## src/Lucene.Net.Tests/Search/TestFieldCache.cs: ########## @@ -107,9 +107,9 @@ public override void TearDown() // LUCENENET: Changed to non-static because NewIndexWriterConfig is non-static Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/Spans/TestFieldMaskingSpanQuery.cs: ########## @@ -62,9 +62,9 @@ protected internal static Field GetField(string name, string value) /// Is non-static because NewIndexWriterConfig is no longer static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests.Facet/Taxonomy/TestTaxonomyFacetAssociations.cs: ########## @@ -53,9 +53,9 @@ public class TestTaxonomyFacetAssociations : FacetTestCase /// Is non-static because Similarity and TimeZone are not static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestMultiTermQueryRewrites.cs: ########## @@ -52,9 +52,9 @@ public class TestMultiTermQueryRewrites : LuceneTestCase /// Is non-static because Similarity and TimeZone are not static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/Spans/TestBasics.cs: ########## @@ -102,9 +102,9 @@ public override void Reset() /// Is non-static because NewIndexWriterConfig is no longer static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestNGramPhraseQuery.cs: ########## @@ -38,9 +38,9 @@ public class TestNGramPhraseQuery : LuceneTestCase /// Is non-static because Similarity and TimeZone are not static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestMinShouldMatch2.cs: ########## @@ -67,9 +67,9 @@ public class TestMinShouldMatch2 : LuceneTestCase /// Is non-static because Similarity and TimeZone are not static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestBooleanMinShouldMatch.cs: ########## @@ -50,9 +50,9 @@ public class TestBooleanMinShouldMatch : LuceneTestCase /// Is non-static because NewStringField is no longer static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestNumericRangeQuery64.cs: ########## @@ -67,9 +67,9 @@ public class TestNumericRangeQuery64 : LuceneTestCase /// Is non-static because NewIndexWriterConfig is no longer static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestTermVectors.cs: ########## @@ -53,9 +53,9 @@ public class TestTermVectors : LuceneTestCase /// Is non-static because NewIndexWriterConfig is no longer static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestNumericRangeQuery32.cs: ########## @@ -67,9 +67,9 @@ public class TestNumericRangeQuery32 : LuceneTestCase /// Is non-static because NewIndexWriterConfig is no longer static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. ########## src/Lucene.Net.Tests/Search/TestPrefixInBooleanQuery.cs: ########## @@ -51,9 +51,9 @@ public class TestPrefixInBooleanQuery : LuceneTestCase /// Is non-static because Similarity and TimeZone are not static. Review Comment: Please remove this comment. It was added during the attempt to use xUnit and no longer reflects the reason why this method is not static (due to NUnit calling it in the wrong order). We already explain that in `LuceneTestCase`, so I don't see a need to repeat it on every overload. -- 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