You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by me...@apache.org on 2018/07/27 17:47:03 UTC
[beam-site] 01/05: Add post-commit tests policies summary page.
This is an automated email from the ASF dual-hosted git repository.
mergebot-role pushed a commit to branch mergebot
in repository https://gitbox.apache.org/repos/asf/beam-site.git
commit a278295a04a66b8da720eed392572ae0bdacf66c
Author: Mikhail Gryzykhin <mi...@google.com>
AuthorDate: Tue Jul 24 14:10:04 2018 -0700
Add post-commit tests policies summary page.
---
src/_includes/section-menu/contribute.html | 6 ++
src/contribute/postcommits-guides.md | 80 ++++++++++++++++++++++
src/contribute/postcommits-policies-details.md | 93 ++++++++++++++++++++++++++
src/contribute/postcommits-policies.md | 82 +++++++++++++++++++++++
src/contribute/testing.md | 81 ++++++++++++++++++++++
5 files changed, 342 insertions(+)
diff --git a/src/_includes/section-menu/contribute.html b/src/_includes/section-menu/contribute.html
index 37e510c..07affbc 100644
--- a/src/_includes/section-menu/contribute.html
+++ b/src/_includes/section-menu/contribute.html
@@ -34,6 +34,12 @@
</ul>
</li>
<li>
+ <span class="section-nav-list-title">Policies</span>
+ <ul class="section-nav-list">
+ <li><a href="{{ site.baseurl }}/contribute/postcommits-policies/">Post-commit tests policies</a></li>
+ </ul>
+</li>
+<li>
<span class="section-nav-list-title">Committers</span>
<ul class="section-nav-list">
diff --git a/src/contribute/postcommits-guides.md b/src/contribute/postcommits-guides.md
new file mode 100644
index 0000000..efb91a6
--- /dev/null
+++ b/src/contribute/postcommits-guides.md
@@ -0,0 +1,80 @@
+---
+layout: section
+title: 'Post-commit tests processes guides'
+permalink: /contribute/postcommits-guides/
+section_menu: section-menu/contribute.html
+---
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Post-commit tests processes guides
+
+[TOC]
+
+
+## Finding person to fix post-commit tests failures {#find_specialist}
+
+Finding proper person to triage the test failure might not be a trivial task.
+However there are some guidelines.
+
+1. If you can do it -- go for it.
+1. Check GitHub blame on files with problematic code.
+1. Reach out to
+ [Slack chat](https://the-asf.slack.com/messages/C9H0YNP3P/apps/A0F7VRFKN/).
+1. Reach out to dev@beam.apache.org.
+
+
+## Rolling back commit {#rollback}
+
+Most likely, you are on this page because there is a failing post-commit test
+and you want to rollback culprit commit.
+
+Since rolling back someones change might be inconvenient for the person that
+made the original change, we ask you to do some extra steps. These steps are
+intended to help that person fix the issue that caused test failure and get his
+feature back in line.
+
+1. Rollback the PR
+1. Create JIRA task with information on why the code rolled back, links to
+ corresponding Tests failure task, triage information, any other relevant
+ information. Assign this task to original PR author.
+1. Consider re-opening Jira ticket that was closed by original PR if any.
+1. Send notification email with information of roll-back, links to original PR,
+ roll-back PR, reasons for roll-back to:
+ * dev@beam.apache.org
+ * Original PR author and committer of that PR.
+1. Close test-failure Jira task. Your work is done here.
+
+
+## Disabling failing test {#disabling}
+
+Usually we want our tests green. If they eventually turn red, we fix them or do
+rollback.
+
+However sometimes it might take us too much time to fix some specific test. In
+this situation it might be feasible to disable a single test, so that the rest
+of the suite give valid health signal.
+
+However this should be done with great caution, since disabling a test means
+that we build upon shaky, not fully tested foundation.
+
+Because of this we want to do some extra precautions if we decide to follow this
+path:
+
+* Notify everyone on dev list that some test is being disabled and describe
+ the problem.
+* It is everyones job at this point to avoid pushing more code to the area
+ that failing test covered, since this code will not be properly tested at
+ this point.
+* Do the fix and get the test back in line as soon as possible.
diff --git a/src/contribute/postcommits-policies-details.md b/src/contribute/postcommits-policies-details.md
new file mode 100644
index 0000000..6de0258
--- /dev/null
+++ b/src/contribute/postcommits-policies-details.md
@@ -0,0 +1,93 @@
+---
+layout: section
+title: 'Post-commit policies details'
+permalink: /contribute/postcommits-policies-details/
+section_menu: section-menu/contribute.html
+---
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Post-commit policies details
+
+A post-commit test failure means that there is a bug in the code. The longer the
+bug exists, the harder it is to fix it due to ongoing code contributions. As a
+result, we want to fix bugs quickly. The Beam community's post-commit test
+policies help keep our code and test results in a good state.
+
+
+## Rollback first {#rollback_first}
+
+Beam uses a "rollback first" approach: the first action to resolve a test
+failure is to rollback the culprit code change. The two main benefits of this
+approach are short implementation time and high reliability. When we rollback
+first, we quickly return to a previously verified good state.
+
+At a high level, this approach consists of the following steps:
+
+1. Revert the culprit commit.
+1. Re-run the post-commit tests to verify the tests pass.
+1. Push the revert commit.
+
+For background on this policy, see the
+[mailing list thread](https://lists.apache.org/thread.html/3bb4aa777751da2e2d7e22666aa6a2e18ae31891cb09d91718b75e74@%3Cdev.beam.apache.org%3E)
+and [design doc](https://docs.google.com/document/d/1sczGwnCvdHiboVajGVdnZL0rfnr7ViXXAebBAf_uQME/edit).
+
+
+## A failing test is a critical/P1 issue {#failing_test_is_critical_bug}
+
+It is difficult to properly verify new changes made on top of buggy code. In
+some cases, adding additional code can make the problem worse. To avoid this
+situation, fixing failing tests is our highest priority.
+
+
+## A flaky test is a critical/P1 issue {#flake_is_failing}
+
+Flaky tests are considered failing tests, and fixing a flaky test is a
+critical/P1 issue.
+
+Flaky tests are tests that randomly succeed or fail while using the same code
+version. Flaky test failures are one of the most dangerous types of failures
+because they are easy to ignore -- another run of the flaky test might pass
+successfully. However, these failures can hide real bugs and flaky tests often
+slowly accumulate. Someone must repeatedly triage the failures, and flaky tests
+are often the hardest ones to fix.
+
+Flaky tests do not provide a reliable quality signal, so it is important to
+quickly fix the flakiness. If a fix will take awhile to implement, it is safer
+to disable the test until the fix is ready.
+
+Martin Fowler has a good [article](https://martinfowler.com/articles/nonDeterminism.html)
+about non-determinism in tests.
+
+
+## Flaky tests must be fixed or removed {#remove_flake}
+
+Flaky tests do not provide a reliable quality signal, which has a harmful effect
+on all tests and can lead to a loss of trust in our test suite. As a result,
+contributors might start to ignore test failures.
+
+We want everyone to trust our tests, so it is important to diligently fix all
+flaky tests. If it is not possible to fix a flaky test, we must remove the test.
+
+
+## Add new pre-commit tests as part of a post-commit fix {#precommit_for_postcommit}
+
+Post-commit tests are an important fail-safe, but we want to fail fast. Failing
+fast means that we want to detect bugs in pre-commit tests, and _not_ in
+post-commit tests.
+
+When you implement a fix for a post-commit test failure, add a new pre-commit
+test that will detect similar failures in the future. For example, you can
+implement a new unit test that covers a problematic code branch.
+
diff --git a/src/contribute/postcommits-policies.md b/src/contribute/postcommits-policies.md
new file mode 100644
index 0000000..bd6609c
--- /dev/null
+++ b/src/contribute/postcommits-policies.md
@@ -0,0 +1,82 @@
+---
+layout: section
+title: 'Post-commit tests policies'
+section_menu: section-menu/contribute.html
+permalink: /contribute/postcommits-policies/
+---
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Post-commit tests policies
+
+Post-commit tests validate that Beam works correctly in a live environment. The
+tests also catch errors that are hard to predict in the design and
+implementation stages
+
+Even though post-commit tests run after the code is merged into the repository,
+it is important that the tests pass reliably. Jenkins executes post-commit tests
+against the HEAD of the master branch. If post-commit tests fail, there is a
+problem with the HEAD build. In addition, post-commit tests are time consuming
+to run, and it is often hard to triage test failures.
+
+
+## Policies {#policies}
+
+To ensure that Beam's post-commit tests are reliable and healthy, the Beam
+community follows these post-commit test policies:
+
+* [Rollback first]({{ site.baseurl }}/contribute/postcommits-policies-details/index.html#rollback_first)
+* [A failing test is a critical bug]({{ site.baseurl }}/contribute/postcommits-policies-details/index.html#failing_test_is_critical_bug)
+* [A flaky test is a critical bug]({{ site.baseurl }}/contribute/postcommits-policies-details/index.html#flake_is_failing)
+* [Flaky tests must either be fixed or removed]({{ site.baseurl }}/contribute/postcommits-policies-details/index.html#remove_flake)
+* [Fixes for post-commit failures should include a corresponding new pre-commit test]({{ site.baseurl }}/contribute/postcommits-policies-details/index.html#precommit_for_postcommit)
+
+
+## Post-commit test failure scenarios
+
+When a post-commit test fails, follow the provided steps for your situation.
+
+### I found a test failure {#found-failing-test}
+
+1. Create a [JIRA issue](https://issues.apache.org/jira/issues/?jql=project%20%3D%20BEAM%20AND%20component%20%3D%20test-failures)
+ and assign it to yourself.
+1. Do high level triage of the failure.
+1. [Assign the JIRA issue to a relevant person]({{ site.baseurl }}/contribute/postcommits-guides/index.html#find_specialist).
+
+### I was assigned a JIRA issue for a test failure {#assigned-failing-test}
+
+1. [Rollback the culprit change]({{ site.baseurl }}/contribute/postcommits-guides/index.md#rollback).
+1. If you determine that rollback will take longer than 8 hours, [disable the
+ test temporarily]({{ site.baseurl }}/contribute/postcommits-guides/index.md#disabling) while you rollback or create a
+ fix.
+
+> Note: Rollback is always the first course of action. If a fix is trivial,
+> open a pull request with the proposed fix while doing rollback.
+
+### My change was rolled back due to a test failure {#pr-rolled-back}
+
+1. Look at the JIRA issue to find the reason for the rollback.
+1. Fix your code and re-run the post-commit tests.
+1. Implement new pre-commit tests that will catch similar bugs before future
+ code is merged into the repository.
+1. Open a new PR that contains your fix and the new pre-commit tests.
+
+## Useful links
+
+* [Best practices for writing tests]({{ site.baseurl }}/contribute/testing/index.md#best_practices)
+
+## References
+
+1. [Keeping post-commit tests green](https://lists.apache.org/thread.html/3bb4aa777751da2e2d7e22666aa6a2e18ae31891cb09d91718b75e74@%3Cdev.beam.apache.org%3E)
+ mailing list proposal thread.
diff --git a/src/contribute/testing.md b/src/contribute/testing.md
index 5002227..5fdd9ac 100644
--- a/src/contribute/testing.md
+++ b/src/contribute/testing.md
@@ -561,3 +561,84 @@ classes which reside under `"org[.]apache[.]beam[.].*Test.*"`,
`"org[.]apache[.]beam[.].*IT"` or `"java[.]lang.*"`, belong to neither
of the packages: `org.apache.beam.x`, `org.apache.beam.y`, `org.apache.beam.z`,
nor equal to `Other.class`.
+
+## Best practices for writing tests {#best_practices}
+
+The following best practices help you to write reliable and maintainable tests.
+
+### Aim for one failure path
+
+An ideal test has one failure path. When you create your tests, minimize the
+possible reasons for a test failure. A developer can debug a problem more
+easily when there are fewer failure paths.
+
+### Avoid non-deterministic code
+
+Reliable tests are predictable and deterministic. Tests that contain
+non-deterministic code are hard to debug and are often flaky. Non-deterministic
+code includes the use of randomness, time, and multithreading.
+
+To avoid non-deterministic code, mock the corresponding methods or classes.
+
+### Use descriptive test names
+
+Helpful test names contain details about your test, such as test parameters and
+the expected result. Ideally, a developer can read the test name and know where
+the buggy code is and how to reproduce the bug.
+
+An easy and effective way to name your methods is to use these three questions:
+
+* What you are testing?
+* What are the parameters of the test?
+* What is the expected result of the test?
+
+For example, consider a scenario where you want to add a test for the
+`Divide` method:
+
+```java
+float Divide(float dividend, float divisor) {
+ return dividend / divisor;
+}
+
+...
+
+@Test
+void <--TestMethodName-->() {
+ assertThrows(Divide(10, 0))
+}
+```
+
+If you use a simple test name, such as `testDivide()`, you are missing important
+information such as the expected action, parameter information, and expected
+test result. As a result, triaging a test failure requires you to look at the
+test implementation to see what the test does.
+
+Instead, use a name such as `invokingDivideWithDivisorEqualToZeroThrowsException()`,
+which specifies:
+
+* the expected action of the test (`invokingDivide`)
+* details about important parameters (the divisor is zero)
+* the expected result (the test throws an exception)
+
+If this test fails, you can look at the descriptive test name to find the most
+probable cause of the failure. In addition, test frameworks and test result
+dashboards use the test name when reporting test results. Descriptive names
+enable contributors to look at test suite results and easily see what
+features are failing.
+
+Long method names are not a problem for test code. Test names are rarely used
+(usually when you triage and debug), and when you do need to look at a
+test, it is helpful to have descriptive names.
+
+
+### Use a pre-commit test if possible
+
+Post-commit tests validate that Beam works correctly in broad variety of
+scenarios. The tests catch errors that are hard to predict in the design and
+implementation stages
+
+However, we often write a test to verify a specific scenario. In this situation,
+it is usually possible to implement the test as a unit test or a component test.
+You can add your unit tests or component tests to the pre-commit test suite, and
+the pre-commit test results give you faster code health feedback during the
+development stage, when a bug is cheap to fix.