Skip to content

Skip clean_removed test#9207

Merged
ruflin merged 1 commit intoelastic:masterfrom
ruflin:skip-clean_removed
Nov 22, 2018
Merged

Skip clean_removed test#9207
ruflin merged 1 commit intoelastic:masterfrom
ruflin:skip-clean_removed

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented Nov 22, 2018

See #7690

@ruflin ruflin added review flaky-test Unstable or unreliable test cases. labels Nov 22, 2018
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.

Add a comment mentioning the reason why the test is skipped? or with a link to the issue?

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.

Good idea, done.

Copy link
Copy Markdown
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

instead of raising SkipTest, please use the decorator and add link to #7690

import unittest
@unittest.skip("temporarily disabled")

@ruflin ruflin force-pushed the skip-clean_removed branch from 6e0d544 to 26101de Compare November 22, 2018 18:27
@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Nov 22, 2018

@ph Can do, but will it make any difference?

@ruflin ruflin force-pushed the skip-clean_removed branch from 26101de to 95c8c72 Compare November 22, 2018 18:38
@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Nov 22, 2018

@ph Changed.

@ph
Copy link
Copy Markdown
Contributor

ph commented Nov 22, 2018

@ph Can do, but will it make any difference?

it is just cleaner IMHO :)

Copy link
Copy Markdown
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM

@ph ph added the needs_backport PR is waiting to be backported to other branches. label Nov 22, 2018
@ph
Copy link
Copy Markdown
Contributor

ph commented Nov 22, 2018

@ruflin I have added needs_backport label, I think we want to disable that in all the current branches.

@ruflin ruflin merged commit be187b9 into elastic:master Nov 22, 2018
@ruflin ruflin deleted the skip-clean_removed branch November 22, 2018 20:42
ruflin added a commit to ruflin/beats that referenced this pull request Nov 22, 2018
@ruflin ruflin added v6.6.0 and removed needs_backport PR is waiting to be backported to other branches. labels Nov 22, 2018
@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Nov 22, 2018

backported to 6.x. Would ignore it for now in 6.5.

else:
assert data[0]["offset"] == 2

@unittest.skip("Skipped as flaky: https://github.com/elastic/beats/issues/7690")
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.

👍

"""
Checks that files which were removed, the state is removed
"""

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.

Nit. Unneeded space added.

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.

nit ignored as already merged :-)

ph pushed a commit that referenced this pull request Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flaky-test Unstable or unreliable test cases. review v6.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants