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

Docker build for the revised ITs #12707

Merged
merged 3 commits into from
Aug 10, 2022

Conversation

paul-rogers
Copy link
Contributor

Includes the Docker image build for the revised ITs. Split out from PR #12368 to simplify review and merge.

This PR contains two items:

it-tools which defines the IT test extensions that are added into the IT Docker image. These are the same as what the integration-tests project provides, just built separately and packaged so they can be added to the image. it-tools is built as part of the Maven build, after distribution. Its output is an extension jar. No harm in including it even in non-IT builds.

it-image which builds the Docker image. It is activated by the test-image profile.

In addition, .travis.yml is modified to run the Docker image to show that it builds. Nothing uses the image, that will come when #12368 is merged. We're just proving that that the build works to assure reviewers that things are correct.

The changes here have already been reviewed as part of #12368. This is a subset of those changes. The test themselves, and the documentation, is omitted to keep the PR small.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors. (No, will come later.)
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. (No. Will come later with extensive documentation.)
  • added integration tests. (Not yet, will come later.)

@clintropolis clintropolis added Area - Testing Area - Dev For items related to the project itself, like dev docs and checklists, but not CI labels Jul 15, 2022
@paul-rogers paul-rogers marked this pull request as draft July 22, 2022 04:04
@paul-rogers paul-rogers marked this pull request as ready for review August 4, 2022 21:28
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Thanks for extracting this into a smaller PR, @paul-rogers !
The changes look good to me.

# See the License for the specific language governing permissions and
# limitations under the License.

set -e
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add a one-line description for each of these scripts either in the respective files or in a single README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Added documentation to the scripts in this PR. The markdown files will appear in a future PR: I'll update the list of scripts in those files as well.

@paul-rogers
Copy link
Contributor Author

@kfaraz, thanks much for the final review! The final commit updates script documentation as you suggested. Changing script comments should not break anything, but Travis will do its thing anyway. Let's hope we get lucky and don't hit any flaky tests.

@abhishekagarwal87 abhishekagarwal87 merged commit 4706a4c into apache:master Aug 10, 2022
@abhishekagarwal87 abhishekagarwal87 added this to the 24.0.0 milestone Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area - Dev For items related to the project itself, like dev docs and checklists, but not CI Area - Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants