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

[Testing] feat: ci: Automated suggestion if check-gen job fails #12352

Closed
wants to merge 6 commits into from

Conversation

rjan90
Copy link
Contributor

@rjan90 rjan90 commented Aug 7, 2024

Proposed Changes

Adding a new job that suggests changes if the make docsgen checks fail, since we already are running it

  1. In the check-gen job:

    • Captures git diff output to a patch file before and after running make docsgen-cli.
    • Uploads the patch file as an artifact if the job fails.
  2. Adds a new suggest-codegen-changes job that:

    • Runs only if check-gen fails and the event is a pull request.
    • Downloads the codegen diff artifact.
    • Creates a GitHub PR comment with a suggestion to apply the codegen changes.

Testing:

  • Confirm that the suggest-codegen-changes test gets skipped if check-gen passes.
  • Verify that the check-gen job correctly creates and uploads the patch file on failure.
  • Confirm that the suggest-codegen-changes job runs as expected on PR events with failed checks.
  • Test a PR with outdated codegen to ensure it receives a helpful suggestion comment.

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • Update CHANGELOG.md or signal that this change does not need it.
    • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
    • If the change does not require a CHANGELOG.md entry, do one of the following:
      • Add [skip changelog] to the PR title
      • Add the label skip/changelog to the PR
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

Automated suggestion if make docgen check fails
@rjan90
Copy link
Contributor Author

rjan90 commented Aug 7, 2024

  • Confirm that the suggest-codegen-changes test gets skipped if check-gen passes.

Test correctly gets skipped if check-gen passes:
Screenshot 2024-08-07 at 12 15 22

test: bump version to make `make gen` fail
Resolve unexpected EOF error
@rjan90 rjan90 changed the title [Testing] feat: ci: Automated suggestion if make docgen check fails [Testing] feat: ci: Automated suggestion if check-gen job fails Aug 7, 2024
Add `actions/checkout@v4`
Add write-permissions to PRs
Read the content of the patch file before heredoc
Copy link

github-actions bot commented Aug 7, 2024

failed, suggested fix:

diff --git a/build/openrpc/full.json b/build/openrpc/full.json
index ab31329..327ceaf 100644
--- a/build/openrpc/full.json
+++ b/build/openrpc/full.json
@@ -2,7 +2,7 @@
     "openrpc": "1.2.6",
     "info": {
         "title": "Lotus RPC API",
-        "version": "1.28.2-dev"
+        "version": "1.29.2-dev"
     },
     "methods": [
         {
diff --git a/build/openrpc/gateway.json b/build/openrpc/gateway.json
index c48d4a2..1b9cdd7 100644
--- a/build/openrpc/gateway.json
+++ b/build/openrpc/gateway.json
@@ -2,7 +2,7 @@
     "openrpc": "1.2.6",
     "info": {
         "title": "Lotus RPC API",
-        "version": "1.28.2-dev"
+        "version": "1.29.2-dev"
     },
     "methods": [
         {
diff --git a/build/openrpc/miner.json b/build/openrpc/miner.json
index a2a5ae7..8170629 100644
--- a/build/openrpc/miner.json
+++ b/build/openrpc/miner.json
@@ -2,7 +2,7 @@
     "openrpc": "1.2.6",
     "info": {
         "title": "Lotus RPC API",
-        "version": "1.28.2-dev"
+        "version": "1.29.2-dev"
     },
     "methods": [
         {
diff --git a/build/openrpc/worker.json b/build/openrpc/worker.json
index ed08b04..d39f452 100644
--- a/build/openrpc/worker.json
+++ b/build/openrpc/worker.json
@@ -2,7 +2,7 @@
     "openrpc": "1.2.6",
     "info": {
         "title": "Lotus RPC API",
-        "version": "1.28.2-dev"
+        "version": "1.29.2-dev"
     },
     "methods": [
         {

To apply these changes, please review and manually apply the suggestion above.

@rjan90
Copy link
Contributor Author

rjan90 commented Aug 7, 2024

This was not as straightforward as I hoped - it seems like we have four options here. Given that the make gen diff can span multiple files and the diff can be large, we can either:

  1. Make the bot open a separate pull request with the suggested changes, which can then be merged into the PR.
  2. Automatically commit the changes to the PR when the check-gen fails
  3. Create a workflow which listens for comments on PRs. When it detects a !fixup comment:
    • When the check-gen job fails, it will create a comment with the suggested changes and instructions to use !fixup to apply these.
    • If the user comments !fixup, it will trigger the apply-codegen-changes job, which will automatically apply the changes and push them to the PR.
  4. Drop this

@filecoin-project filecoin-project deleted a comment from github-actions bot Aug 7, 2024
@rjan90
Copy link
Contributor Author

rjan90 commented Nov 22, 2024

Closing as this has been superseded by: #12681

@rjan90 rjan90 closed this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant