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 integration tests with eventdata #968

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit we add integration tests with the eventdata track. As
it is not included in the regular Rally tracks, it uses many features of
Rally and we often use it for benchmarks, it makes sense to integrate
testing more tightly.

Note that we do not add tests for all challenges of the eventdata track
as elastic/rally-eventdata-track#82 will remove some of them.

With this commit we add integration tests with the eventdata track. As
it is not included in the regular Rally tracks, it uses many features of
Rally and we often use it for benchmarks, it makes sense to integrate
testing more tightly.

Note that we do not add tests for all challenges of the eventdata track
as elastic/rally-eventdata-track#82 will remove some of them.
@danielmitterdorfer danielmitterdorfer added enhancement Improves the status quo :misc Changes that don't affect users directly: linter fixes, test improvements, etc. labels Apr 20, 2020
@danielmitterdorfer danielmitterdorfer added this to the 1.5.0 milestone Apr 20, 2020
@danielmitterdorfer danielmitterdorfer self-assigned this Apr 20, 2020
Copy link
Contributor

@hub-cap hub-cap left a comment

Choose a reason for hiding this comment

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

This looks good. One thing Ive noticed is that we add the on-error flag to all the rally invocations, rather than passing it as a defaulted param that will put it in the it.esrally() invocation. I get that we might want to not fail on some errors, but it seems like its something that could be forgotten by a new test creator.

@danielmitterdorfer
Copy link
Member Author

This looks good. One thing Ive noticed is that we add the on-error flag to all the rally invocations, rather than passing it as a defaulted param that will put it in the it.esrally() invocation. I get that we might want to not fail on some errors, but it seems like its something that could be forgotten by a new test creator.

Thanks for the review and good point with the on-error flag. As discussed on another channel let's do this in a separate PR by providing a dedicated method for the invocation of the race subcommand as --on-error=abort is not applicable to every subcommand (like info or list).

@danielmitterdorfer danielmitterdorfer merged commit f375290 into elastic:master Apr 20, 2020
@danielmitterdorfer danielmitterdorfer deleted the migrate-eventdata branch April 20, 2020 14:52
hub-cap added a commit to hub-cap/rally that referenced this pull request Apr 20, 2020
All of the commands that relied on the default rally race command were
manually adding the --on-error flag to the cli. This is something that
can be missed by a developer or reviewer. This commit adds a race()
command so that the it tests can take advantage of any defaults added to
every race in the tests.

Relates elastic#968
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :misc Changes that don't affect users directly: linter fixes, test improvements, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants