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

🧹 Rewrite unit tests in Golang #215

Merged
merged 6 commits into from
Sep 7, 2022
Merged

🧹 Rewrite unit tests in Golang #215

merged 6 commits into from
Sep 7, 2022

Conversation

cardil
Copy link
Contributor

@cardil cardil commented Sep 2, 2022

Changes

  • 🧹 Rewrite unit tests in Golang
  • 🧹 Fix scripts to survive in the set -Eeuo pipefail mode

/kind cleanup

Fixes #61

@knative-prow
Copy link

knative-prow bot commented Sep 2, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@knative-prow knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 2, 2022
@cardil cardil mentioned this pull request Sep 2, 2022
Merged
@cardil
Copy link
Contributor Author

cardil commented Sep 2, 2022

/test all

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
@cardil
Rewrite the unit tests to Golang
ab80491 
Signed-off-by: Chris Suszyński <ksuszyns@redhat.com>
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
@cardil cardil marked this pull request as ready for review September 2, 2022 15:40
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2022
@cardil
Copy link
Contributor Author

cardil commented Sep 2, 2022

kubernetes/test-infra#27358 blocks this PR

/hold

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 2, 2022
@cardil cardil marked this pull request as draft September 2, 2022 17:12
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2022
@cardil cardil changed the title 🧹 Rewrite unit tests in Golang [WIP] 🧹 Rewrite unit tests in Golang Sep 2, 2022
@cardil
Copy link
Contributor Author

cardil commented Sep 6, 2022

/unhold

kubernetes/test-infra#27358 wasn't the cause of the unit test failure.

It was due to:

hack/library.sh

Line 70 in f9dc722

(( IS_PROW )) && exec 2>&1

It was fixed in 8ba8bc0

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 6, 2022
@cardil cardil changed the title [WIP] 🧹 Rewrite unit tests in Golang 🧹 Rewrite unit tests in Golang Sep 6, 2022
@cardil cardil marked this pull request as ready for review September 6, 2022 16:02
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 6, 2022
@cardil
Copy link
Contributor Author

cardil commented Sep 6, 2022

/assign @kvmware
/assign @upodroid
/cc @mgencur

@knative-prow knative-prow bot requested a review from mgencur September 6, 2022 16:02
@cardil cardil force-pushed the rewrite/unit-test-in-go branch 4 times, most recently from 4268e95 to d6c8af0 Compare September 6, 2022 17:31
@cardil
Cleanup before review
0675a9c 
Signed-off-by: Chris Suszyński <ksuszyns@redhat.com>
@cardil
Copy link
Contributor Author

cardil commented Sep 6, 2022

/test unit-tests_hack_main

@cardil
Copy link
Contributor Author

cardil commented Sep 7, 2022

I'd like this merged, before I could work on #221

upodroid
upodroid approved these changes Sep 7, 2022
View reviewed changes
Copy link
Member

@upodroid upodroid left a comment

Choose a reason for hiding this comment

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

Thanks

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 7, 2022
@knative-prow
Copy link

knative-prow bot commented Sep 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, upodroid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot merged commit 6c30196 into knative:main Sep 7, 2022
@cardil cardil deleted the rewrite/unit-test-in-go branch September 7, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@upodroid upodroid upodroid approved these changes

@mgencur mgencur Awaiting requested review from mgencur

Assignees

@upodroid upodroid

@krsna-m krsna-m

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Scripts should survive set -Eeuo pipefail mode
4 participants
@cardil @upodroid @knative-prow-robot @krsna-m