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

Add debug code to flaky field_data test #15535

Merged
merged 2 commits into from Mar 28, 2018
Merged

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Dec 11, 2017

Gathering more info for #13163

This PR re-enables a flaky test and adds some code to log additional debug info the next time it fails. As I mentioned in a comment in the linked issue, I suspect there's an actual bug in our response handling or _source formatting code. I dug through some of the code that I thought might be responsible but I didn't see any obvious issues. So before blindly searching any further I want to see the actual ES response for one of these failures and be 100% sure that the issue isn't with the data ES is sending back. If the ES data is fine I may need to figure out another way to zero in on the issue.

@weltenwort
Copy link
Member

The current CI failure seems to be something else... jenkins, test this

@stacey-gammon
Copy link
Contributor

@Bargs did you give up on this PR or you still looking for a review for it?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Bargs
Copy link
Contributor Author

Bargs commented Feb 20, 2018

@stacey-gammon I was waiting for review, then forgot about it :)

I dunno if this test has continued to be flaky in the intervening time or not

@stacey-gammon stacey-gammon removed their request for review March 8, 2018 17:05
@Bargs
Copy link
Contributor Author

Bargs commented Mar 16, 2018

Ping @stacey-gammon @weltenwort I've rebased this PR, please take a look if you get a chance.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Could not reproduce the targeted failure locally, but this looks like it could give some insight.

Would be sweet if Kibana offered a hook for the tests to gather the request details. Alternatively, we could inject some JS via driver.executeScript to hook into the xhr requests ourselves or communicate with the devtools to save the HAR. The former would be more compatible across browsers, I assume.

Anyway, that's for a different PR 😉

@stacey-gammon
Copy link
Contributor

did you confirm that a failure will spit out the debug info? I tried to simulate a failure, but I get:

screen shot 2018-03-19 at 9 49 20 am

instead of the debug output. But maybe something funky on my local. (btw if you run into other issues running tests locally, you might need to bump chromedriver temporarily).

@Bargs
Copy link
Contributor Author

Bargs commented Mar 19, 2018

I did confirm it worked, but that was back in 2017 😆

I'll test it out and make sure.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@jimgoodwin jimgoodwin added v6.2.4 and removed v6.2.3 labels Mar 20, 2018
@Bargs
Copy link
Contributor Author

Bargs commented Mar 26, 2018

@stacey-gammon so it looks like the UI changed since I first wrote this, I just had to update a helper to handle that change. You should be able to test the console output by simply changing the expected string now.

@Bargs Bargs added v6.3.0 and removed v6.2.4 labels Mar 26, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@Bargs
Copy link
Contributor Author

Bargs commented Mar 27, 2018

Failing tests don't seem to be related to this change, retrying.

Jenkins, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Bargs Bargs merged commit a89027b into elastic:master Mar 28, 2018
@Bargs Bargs removed the v6.3.0 label Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants