Skip to content

[7.0] add tests for fixing cmd apm-server setup template#1935

Merged
simitt merged 2 commits intoelastic:7.0from
simitt:1922-fix-setup-template
Feb 20, 2019
Merged

[7.0] add tests for fixing cmd apm-server setup template#1935
simitt merged 2 commits intoelastic:7.0from
simitt:1922-fix-setup-template

Conversation

@simitt
Copy link
Copy Markdown
Contributor

@simitt simitt commented Feb 15, 2019

  • backports adding tests for setup cmd.

self.es.indices.delete_template('apm-*')
except:
pass
assert self.es.indices.exists_template(name='apm-*') == False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not a big deal, but assert not self.es.indices.exists_template(name='apm-*') would be more idiomatic imo

assert self.es.indices.exists_template(name='apm-*') == False

shutil.copy(self.beat_path + "/_meta/beat.yml",
os.path.join(self.working_dir, "apm-server.yml"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think subclassing test_export.SubCommandTest would eliminate the need for this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to derive from ServerSetUpBaseTest as this calls wait_until_startup() in the setup() command, which again expects to see Starting apm-server in the logs, which won't be there. I did not want to go down the path of overwriting this.

Copy link
Copy Markdown
Member

@graphaelli graphaelli Feb 15, 2019

Choose a reason for hiding this comment

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

SubCommandTest already implements wait_until_started to cover that. I wouldn't insist except that I'm worried about having the config generation in multiple places.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right! I'll change accordingly.

@simitt simitt force-pushed the 1922-fix-setup-template branch 2 times, most recently from 8e19337 to b60fa36 Compare February 18, 2019 15:19
@simitt simitt force-pushed the 1922-fix-setup-template branch from b60fa36 to 456c0ec Compare February 19, 2019 08:17
@simitt simitt merged commit 13367b1 into elastic:7.0 Feb 20, 2019
@simitt simitt deleted the 1922-fix-setup-template branch May 6, 2019 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants