Skip to content

Test: Fix test name#33510

Merged
nik9000 merged 1 commit intoelastic:masterfrom
nik9000:test_fix_name
Sep 7, 2018
Merged

Test: Fix test name#33510
nik9000 merged 1 commit intoelastic:masterfrom
nik9000:test_fix_name

Conversation

@nik9000
Copy link
Copy Markdown
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.

@nik9000
Test: Fix test name
e1f793f 
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 DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. v7.0.0 v6.5.0 labels Sep 7, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Sep 7, 2018

Pinging @elastic/es-core-infra

@rjernst
Copy link
Copy Markdown
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
Copy Markdown
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.

jasontedor
jasontedor approved these changes Sep 7, 2018
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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 mentioned this pull request Sep 7, 2018
Closed
@nik9000 nik9000 merged commit 0685c95 into elastic:master Sep 7, 2018
@nik9000 nik9000 mentioned this pull request Sep 7, 2018
Closed
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 8, 2018
@jasontedor
Merge branch 'master' into auto-follow-license
00f6480 
* 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
@nik9000
Test: Fix test name (#33510)
f366bfa 
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

Reviewers

@jasontedor jasontedor jasontedor approved these changes

Assignees

No one assigned

Labels

:Data Management/Indices APIs DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. >test Issues or PRs that are addressing/adding tests v6.5.0 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nik9000 @elasticmachine @rjernst @jasontedor @colings86