You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2024/11/27 01:08:36 UTC
[PR] SWEEP: Rename BeforeClass/AfterClass to OneTimeSetUp/OneTimeTearDown, #1016 [lucenenet]
paulirwin opened a new pull request, #1051:
URL: https://github.com/apache/lucenenet/pull/1051
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/main/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [X] You've included unit or integration tests for your change, where applicable.
- [X] You've included inline docs for your change, where applicable.
- [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Rename BeforeClass/AfterClass to OneTimeSetUp/OneTimeTearDown to better match NUnit conventions.
Fixes #1016
## Description
See #1016 for details and rationale. This is a breaking change because of the changes in TestFramework.
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SWEEP: Rename BeforeClass/AfterClass to OneTimeSetUp/OneTimeTearDown, #1016 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SWEEP: Rename BeforeClass/AfterClass to OneTimeSetUp/OneTimeTearDown, #1016 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1051:
URL: https://github.com/apache/lucenenet/pull/1051#discussion_r1860856986
##########
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:
Thanks for providing that, I was questioning it.
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SWEEP: Rename BeforeClass/AfterClass to OneTimeSetUp/OneTimeTearDown, #1016 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1051:
URL: https://github.com/apache/lucenenet/pull/1051
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org