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

Unit Testing in Beam Blog Post #32412

Merged
merged 16 commits into from
Sep 12, 2024
Merged

Conversation

svetakvsundhar
Copy link
Contributor

@svetakvsundhar svetakvsundhar commented Sep 9, 2024

Blog post giving opinionated guidance on unittesting in Beam.

Notes

  1. I closed the old PR: Unit Testing in Beam Blog Post #31701.
  2. I refactored the Colab notebook a bit for easier readability with the blog post
  3. I set the release date as today, just to see if we can stage it at any point. I'll modify the release date once the reviewers LGTM the PR.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@svetakvsundhar svetakvsundhar marked this pull request as draft September 9, 2024 17:37
@svetakvsundhar svetakvsundhar marked this pull request as ready for review September 9, 2024 17:48
@svetakvsundhar
Copy link
Contributor Author

R: @damccorm
R: @robertwb

Copy link
Contributor

github-actions bot commented Sep 9, 2024

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

Content-wise this seems good, added a few suggestions

website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
@damccorm
Copy link
Contributor

@rszper would you mind taking a pass as well?

examples/notebooks/blog/unittests_in_beam.ipynb Outdated Show resolved Hide resolved
examples/notebooks/blog/unittests_in_beam.ipynb Outdated Show resolved Hide resolved
examples/notebooks/blog/unittests_in_beam.ipynb Outdated Show resolved Hide resolved
examples/notebooks/blog/unittests_in_beam.ipynb Outdated Show resolved Hide resolved
examples/notebooks/blog/unittests_in_beam.ipynb Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
@svetakvsundhar
Copy link
Contributor Author

Thank you both very much for the thoughtful review and suggestions!

Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

LGTM from a content perspective


For more pointed guidance on testing on Beam/Dataflow, see the [Google Cloud documentation](https://cloud.google.com/dataflow/docs/guides/develop-and-test-pipelines). Additionally, see some more examples of unit testing in Beam [here](https://github.com/apache/beam/blob/736cf50430b375d32093e793e1556567557614e9/sdks/python/apache_beam/ml/inference/base_test.py#L262).
For more guidance about testing on Beam and Dataflow, see the [Google Cloud documentation](https://cloud.google.com/dataflow/docs/guides/develop-and-test-pipelines). For more examples of unit testing in Beam, see [the `base_test.py` code](https://github.com/apache/beam/blob/736cf50430b375d32093e793e1556567557614e9/sdks/python/apache_beam/ml/inference/base_test.py#L262).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For more guidance about testing on Beam and Dataflow, see the [Google Cloud documentation](https://cloud.google.com/dataflow/docs/guides/develop-and-test-pipelines). For more examples of unit testing in Beam, see [the `base_test.py` code](https://github.com/apache/beam/blob/736cf50430b375d32093e793e1556567557614e9/sdks/python/apache_beam/ml/inference/base_test.py#L262).
For more guidance about testing on Beam and Dataflow, see the [Google Cloud documentation](https://cloud.google.com/dataflow/docs/guides/develop-and-test-pipelines). For more examples of unit testing in Beam, see [the base_test.py code](https://github.com/apache/beam/blob/736cf50430b375d32093e793e1556567557614e9/sdks/python/apache_beam/ml/inference/base_test.py#L262).

This renders awkwardly with the backticks

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@rszper rszper left a comment

Choose a reason for hiding this comment

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

Some minor suggestions, otherwise LGTM.

website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
website/www/site/content/en/blog/unit-testing-in-beam.md Outdated Show resolved Hide resolved
svetakvsundhar and others added 4 commits September 12, 2024 18:01
Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
@svetakvsundhar
Copy link
Contributor Author

I'll merge this, should launch by midnight tonight. Thanks again!

@damccorm following up on your previous comment, seems like changing the date to a future date is a way to gate the blog post, I can't see the stage version upon doing so.

@svetakvsundhar svetakvsundhar merged commit 02af7d4 into apache:master Sep 12, 2024
6 checks passed
@svetakvsundhar svetakvsundhar deleted the unittest_blog branch September 13, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants