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

Updated test to retry assertions. Also fixed os specific key binding … #22421

Closed
wants to merge 2 commits into from

Conversation

cuff-links
Copy link
Contributor

Fixes #22230 and #22331

@liza-mae
Copy link
Contributor

@silne30 what do you think about splitting these into two PRs ?

@cuff-links
Copy link
Contributor Author

@liza-mae I thought about that but since the code was one if statement for the handling of the different key binding, seemed like overkill.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@bhavyarm
Copy link
Contributor

@silne30 @liza-mae how do I test this? Just run the PR in my local? Thanks!

@liza-mae
Copy link
Contributor

@bhavyarm thanks for testing this, here are some instructions:
git clone Kibana repo
git checkout -b silne30-silne30/#222230 master
git pull git://github.com/silne30/kibana.git silne30/#222230
Then run the visualization tsvb test. Ping me or John on slack for additional help.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@liza-mae liza-mae left a comment

Choose a reason for hiding this comment

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

The gauge test in this file failed for me when running on cloud. An update to that test is in #22472 let's merge that in here and also update topN label test with the same, since I suspect it will have the same problem. What do you think of using the same coding convention on retry block as other tests. Also do we need to update all the tests to use retry that are doing similar checks?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cuff-links
Copy link
Contributor Author

Duplicate of #22521

@cuff-links cuff-links marked this as a duplicate of #22521 Jan 9, 2019
@cuff-links cuff-links closed this Jan 9, 2019
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.

4 participants