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.

Sign up for GitHub

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

Jump to bottom

Rename example test files, and add requirement for new recipes. #181

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Rename example test files, and add requirement for new recipes. #181

merged 2 commits into from
Nov 13, 2023

Conversation

sawsa307
Copy link
Contributor

@sawsa307 sawsa307 commented Nov 3, 2023 •
edited
Loading

  • Rename test_example and its files to avoid cleanup.sh template being picked up by cleanup_all.sh.
  • Add warning on break changes will be reverted.
  • Add requirement for new recipe PR, and example for the required output.

@sawsa307 sawsa307 force-pushed the add-test-req branch 2 times, most recently from 309a573 to 8f1f879 Compare November 3, 2023 17:38
bowei
bowei reviewed Nov 13, 2023
View reviewed changes
test/README.md Outdated
@@ -18,6 +18,8 @@ limitations under the License.

The tests here are intended to test the recipes in this repo to make sure it workable and up-to-date.

PLEASE NOTE: All recipe tests are run periodically to ensure they are all passed. If a new recipe or changes to an existing recipe causes test failures, it will be REVERTED.
Copy link
Member

Choose a reason for hiding this comment

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

It may be reverted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

bowei
bowei approved these changes Nov 13, 2023
View reviewed changes
test/README.md Outdated
@@ -75,7 +77,7 @@ To cleanup all tests separately, use the following command from test/:

## Adding a new recipe test

For a new recipe, in addition to its yaml file and REAME.md, it should also include a set of test files to make sure the recipe is functional and up-to-date.
For a new recipe, in addition to its yaml file and REAME.md, it should also include a set of test files to make sure the recipe is functional and up-to-date. In the description section of the pull request, you should also provide the result of `make test` tp show your test is passing and is not breaking any other existing tests. See example in [output-example.txt](./test_example/output_example.txt).
Copy link
Member

Choose a reason for hiding this comment

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

tp show => to show

any other existing => other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@sawsa307
Add explanation for possible change reversion, and PR requirements.
2b70e0c 
* Add warning on break changes will be reverted.
* Add requirement for new recipe PR, and example for the required
  output.
@sawsa307 sawsa307 requested a review from bowei November 13, 2023 20:56
@sawsa307
Copy link
Contributor Author

sawsa307 commented Nov 13, 2023 •
edited
Loading

Hi @bowei I've addressed the typos, could you take another look?

@bowei bowei merged commit d38d64a into GoogleCloudPlatform:main Nov 13, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bowei bowei bowei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sawsa307 @bowei