You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/25 08:46:08 UTC

[GitHub] [lucene-solr] MarcusSorealheis opened a new pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

MarcusSorealheis opened a new pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781


   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Lucene or Solr:
   
   * https://issues.apache.org/jira/projects/LUCENE
   * https://issues.apache.org/jira/projects/SOLR
   
   You will need to create an account in Jira in order to create an issue.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-14777:  Fix an ignore test that is helpful.
   
   LUCENE and SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # Description
   
   There are a number of tests that are annotated `@Ignore` but we still need them. This is a first step to start to chip away at the ignores so that the team is no impacted.
   
   # Solution
   
    Fix an ignore test that is helpful.
   
   # Tests
   
   I am adding a test that was previously ignored.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) 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](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [ ] I have developed this patch against the `master` branch.
   - [ ] I have run `ant precommit` and the appropriate test suite.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] markrmiller commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
markrmiller commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-682046480


   > @markrmiller Is this the right idea for removing ignores?
   
   This is the right idea though, as Tim says, some of these tests may already be okay due to other changes.
   
   But basically, remove the ignores, if they don't pass address the issues, if the test is much slower than 10 seconds total (not just what junit reports) move it to @Nightly.
   
   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] MarcusSorealheis commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-681146332


   @thelabdude That's right. We need to compare the hostname. If the test is no longer broken, then this is fine to close.


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] MarcusSorealheis commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-679910659


   I'm inclined to believe that there is another test that is no longer so flakey that also extends `EmbeddedSolrServerTestBase` but I want to keep the scope limited for reviews because of _other_ challenging tests in the branch atm.


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] thelabdude commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-680171669


   Precommit won't work on this branch as there are many `nocommit` in the code (obviously). 


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] thelabdude commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-680928789


   @MarcusSorealheis thanks for helping out with this branch. The `SolrPingTest` passes for me locally w/o your fixes, which is not surprising for ignored tests as a previous issue may have been resolved by other changes in this very active branch. However, I'm not clear on the reasons for your changes in the `SolrZkServer` class? For instance, `server.addr` is a `InetSocketAddress` so doing an `equals` on that to a String is not what we want here:
   ```
   -      if (server.addr.getHostName().equals(myHost)) {
   +      if (server.addr.equals(myHost)) {
   ```


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] MarcusSorealheis closed pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
MarcusSorealheis closed pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781


   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] MarcusSorealheis commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-679892656


   @markrmiller Is this the right idea for removing ignores?


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] MarcusSorealheis commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-679893584


   ![image](https://user-images.githubusercontent.com/2353608/91153456-1371ab00-e675-11ea-859b-7e376fc2a4f9.png)
   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] MarcusSorealheis commented on pull request #1781: SOLR-14777: Fix an ignore test that is helpful.

Posted by GitBox <gi...@apache.org>.
MarcusSorealheis commented on pull request #1781:
URL: https://github.com/apache/lucene-solr/pull/1781#issuecomment-679894298


   I am seeing issues on pre-commit but I did not change any of those files. They should be fine: 
   ```
   BUILD FAILED
   /home/runner/work/lucene-solr/lucene-solr/build.xml:128: Found 489 violations in source files (invalid logger name [log, uses static class name, not specialized logger], invalid logging pattern [not private static final, uses static class name], nocommit).
   ```


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org