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

Test: Fix test name #33510

Merged
merged 1 commit into from
Sep 7, 2018
Merged

Test: Fix test name #33510

merged 1 commit into from
Sep 7, 2018

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Sep 7, 2018

This test has the wrong name and hasn't been automatically running. This
fixes the name so we'll run it.

This test has the wrong name and hasn't been automatically running. This
fixes the name so we'll run it.
@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests review :Data Management/Indices APIs APIs to create and manage indices and templates v7.0.0 v6.5.0 labels Sep 7, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst
Copy link
Member

rjernst commented Sep 7, 2018

Why didn't the naming conventions check find this? Are we missing logic in there for ESRestTestCase?

@nik9000
Copy link
Member Author

nik9000 commented Sep 7, 2018

Why didn't the naming conventions check find this? Are we missing logic in there for ESRestTestCase?

I'll investigate that but I figured getting the test running is worthy of its own PR to get it in more quickly.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@nik9000
Copy link
Member Author

nik9000 commented Sep 7, 2018

Are we missing logic in there for ESRestTestCase?

We are indeed. Right now we have

    /**
     * Superclass for all tests.
     */
    @Input
    private String testClass = "org.apache.lucene.util.LuceneTestCase";
    /**
     * Superclass for all integration tests.
     */
    @Input
    private String integTestClass = "org.elasticsearch.test.ESIntegTestCase";

This is the right configuration for server, but not really for other places. If someone extends ESRestTestCase then it just counts as a regular test. For the most part it'll just fail when it runs during the tests phase. But if we disable the test phase and only have integTest enabled then we never run the rest test and we never know that it is wrong. I'll open an issue.

@nik9000 nik9000 merged commit 0685c95 into elastic:master Sep 7, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 8, 2018
* master: (30 commits)
  Include fallback settings when checking dependencies (elastic#33522)
  [DOCS] Fixing formatting issues in breaking changes
  CRUD: Disable wait for refresh tests with delete
  Test: Fix test name (elastic#33510)
  HLRC: split ingest request converters (elastic#33435)
  Logging: Configure the node name when we have it (elastic#32983)
  HLRC: split xpack request converters (elastic#33444)
  HLRC: split watcher request converters (elastic#33442)
  HLRC: add enable and disable user API support (elastic#33481)
  [DOCS] Fixes formatting error
  TEST: Ensure merge triggered in _source retention test (elastic#33487)
  [ML] Add a file structure determination endpoint (elastic#33471)
  HLRC: ML Forecast Job (elastic#33506)
  HLRC: split migration request converters (elastic#33436)
  HLRC: split snapshot request converters (elastic#33439)
  Make Watcher validation message copy/pasteable
  Removes redundant test method in SQL tests (elastic#33498)
  HLRC: ML Post Data (elastic#33443)
  Pass Directory instead of DirectoryService to Store (elastic#33466)
  Collapse package structure for metrics aggs (elastic#33463)
  ...
nik9000 added a commit that referenced this pull request Sep 25, 2018
This test has the wrong name and hasn't been automatically running. This
fixes the name so we'll run it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >test Issues or PRs that are addressing/adding tests v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants