Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Jun 25, 2017

You can continue a test started in a previous snippet by marking the
next snippet with // TEST[continued]. The trouble is, if you mark the
first snippet in a file with // TEST[continued] you'd get difficult
to predict behavior because you'd continue the test started in another
file. This will usually create a test that fails the build but it isn't
easy to track down what you did wrong. This commit catches this
scenario up front and fails the build with a useful error message.

Similarly, if you put // TEST[continued] directly after a
// TESTSETUP section then the docs tests will fail to run but the
error message did not point you to the // TEST[continued] snippet.
This commit catches this scenario up front as well and fails the build
with a useful error message.

You can continue a test started in a previous snippet by marking the
next snippet with `// TEST[continued]`. The trouble is, if you mark the
first snippet in a file with `// TEST[continued]` you'd get difficult
to predict behavior because you'd continue the test started in another
file. This will usually create a test that fails the build but it isn't
easy to track down what you did wrong. This commit catches this
scenario up front and fails the build with a useful error message.

Similarly, if you put `// TEST[continued]` directly after a
`// TESTSETUP` section then the docs tests will fail to run but the
error message did not point you to the `// TEST[continued]` snippet.
This commit catches this scenario up front as well and fails the build
with a useful error message.
@nik9000 nik9000 added :Delivery/Build Build or test infrastructure v5.6.0 v6.0.0 labels Jun 25, 2017
@nik9000
Copy link
Member Author

nik9000 commented Jun 25, 2017

@lcawl and @tbrooks8, this would have caught the error that we talked about on Friday. And it'd catch an error that I've had a few times that is similarly difficult to track down. And it'd spit out a much more obvious error message.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM

@nik9000
Copy link
Member Author

nik9000 commented Jun 26, 2017

Thanks for reviewing @cbuescher!

@nik9000 nik9000 merged commit 4306315 into elastic:master Jun 26, 2017
nik9000 added a commit that referenced this pull request Jun 26, 2017
You can continue a test started in a previous snippet by marking the
next snippet with `// TEST[continued]`. The trouble is, if you mark the
first snippet in a file with `// TEST[continued]` you'd get difficult
to predict behavior because you'd continue the test started in another
file. This will usually create a test that fails the build but it isn't
easy to track down what you did wrong. This commit catches this
scenario up front and fails the build with a useful error message.

Similarly, if you put `// TEST[continued]` directly after a
`// TESTSETUP` section then the docs tests will fail to run but the
error message did not point you to the `// TEST[continued]` snippet.
This commit catches this scenario up front as well and fails the build
with a useful error message.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 26, 2017
* master:
  Throw useful error on bad docs snippets (elastic#25389)
  Remove `mapping.single_type` from parent join test (elastic#25391)
  Tests: Fix array out of bounds exception in TemplateUpgradeServiceIT
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v5.6.0 v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants