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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
