Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LUCENE-8626: Standardize Lucene Test Files #2026

Merged

Conversation

MarcusSorealheis
Copy link
Contributor

@MarcusSorealheis MarcusSorealheis commented Oct 25, 2020

Description

Janitor here, moving a closer to a standard test file naming convention, starting with Lucene.

Solution

Janitorial work based on the work that @cpoerschke started a couple years ago in the ticket from the title.Almost every change was identical. More to come. PR was already growing too big.

Tests

These are all tests.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the master branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Ref Guide (for Solr changes only).

@ErickErickson
Copy link

LGTM. We can check these in piecemeal rather than do them all at once, as you say the PRs get massive...

@dweiss
Copy link
Contributor

dweiss commented Oct 25, 2020

Wouldn't it be better to add test convention enforcement first, followed by gradual rename of all convention-excluded files? I filed a suggestion on how this could be done - it doesn't have to be that particular one; can be anything but would be better than nothing?

@MarcusSorealheis
Copy link
Contributor Author

@dweiss I'm of the opinion that they could happen in parallel because we know in which direction we are going given the current state. The enforcement will obviously be a plus, but both the enforcement and the renaming are both in flight and gradually landing.

@dweiss
Copy link
Contributor

dweiss commented Oct 25, 2020

ok.

import org.locationtech.spatial4j.context.SpatialContext;
import org.locationtech.spatial4j.shape.Shape;

public class TestQueryEqualsHashCode extends LuceneTestCase {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you intend to add this as a duplicate file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the duplicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intended to remove the previous file lucene/spatial-extras/src/test/org/apache/lucene/spatial/QueryEqualsHashCodeTest.java

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I didn't remove the original in that case good catch

Copy link
Contributor

@CaoManhDat CaoManhDat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!, Gonna commit it after waiting for a day.

@CaoManhDat CaoManhDat merged commit 57729c9 into apache:master Oct 30, 2020
@CaoManhDat
Copy link
Contributor

CaoManhDat commented Oct 30, 2020

Thank you @MarcusSorealheis and everyone

msfroh pushed a commit to msfroh/lucene-solr that referenced this pull request Nov 18, 2020
epugh pushed a commit to epugh/lucene-solr-1 that referenced this pull request Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants