Skip to content

Conversation

@LesnyRumcajs
Copy link
Member

@LesnyRumcajs LesnyRumcajs commented Nov 3, 2025

Summary of changes

Changes introduced in this pull request:

  • Run certain integration tests only on schedule/release/dispatch; it should be straightforward to move some other tests to that land as well,
  • This should save some resources and reduce CI times,
  • Added a Release label step to the release checklist,
  • I think this will work, but as usual, yaml engineering might have some hiccups.

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

  • New Features

    • Integration tests now run on a weekly schedule automatically.
    • Workflow creates integration test issues on main when failures occur, using a standardized template.
    • Additional test job added to surface extra checks and set workflow links for failures.
  • Documentation

    • Release checklist updated to require a Release label on pull requests and removes the previous manual-test note.

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner November 3, 2025 10:21
@LesnyRumcajs LesnyRumcajs requested review from elmattic and hanabi1224 and removed request for a team November 3, 2025 10:21
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Adds an integration-tests issue template, updates the CI workflow (.github/workflows/forest.yml) to add a weekly schedule, conditional gating and an extra_tests job that can create issues on main, and updates the release checklist to require the Release label and remove a manual-test note.

Changes

Cohort / File(s) Summary
GitHub issue template
.github/INTEGRATION_TESTS_ISSUE_TEMPLATE.md
Adds a new issue template with YAML front matter (title "[automated] Integration test failure", label Bug) and a Description section directing users to check workflow logs.
CI workflow
.github/workflows/forest.yml
Adds weekly cron trigger; keeps workflow_dispatch. Introduces conditional gating (schedule / workflow_dispatch / PR with Release label) for jobs, converts/renames calibnet-export-check to gated calibnet-export-check-v1, removes it from integration umbrella needs, and adds a new extra_tests job that depends on calibnet-export-check-v1 and can create an integration-tests issue on main using the new template.
Release docs
documentation/src/developer_documentation/release_checklist.md
Requires Pull Requests to have the Release label in the "Prepare the release" section and removes the prior note about running TEST_PLAN.md manual test steps.

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler as Weekly cron
    participant Dispatcher as Manual dispatch
    participant PR as PR (Release label)
    participant Workflow as forest.yml
    participant Calibnet as calibnet-export-check-v1
    participant ExtraTests as extra_tests
    participant Issue as GitHub Issue API
    participant Main as main branch

    Scheduler->>Workflow: scheduled run
    Dispatcher->>Workflow: manual dispatch
    PR->>Workflow: PR run (with Release label)

    Workflow->>Calibnet: run calibnet-export-check-v1 (only for gated triggers)
    Calibnet-->>Workflow: success/failure

    Workflow->>ExtraTests: run extra_tests (depends on calibnet-export-check-v1)
    ExtraTests-->>Workflow: success/failure

    alt extra_tests fails AND branch == main
        Workflow->>Issue: create issue using `.github/INTEGRATION_TESTS_ISSUE_TEMPLATE.md`
        Issue-->>Workflow: issue created
    else other outcomes
        Workflow-->>Workflow: no issue created
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review focus:
    • .github/workflows/forest.yml: conditional gating logic, job dependency changes, and correct handling of WORKFLOW_URL.
    • Issue creation step: template path, inputs, and branch-only condition.
    • release_checklist.md: ensure the label requirement aligns with workflow gating.

Possibly related PRs

Suggested reviewers

  • hanabi1224
  • elmattic

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'chore: run certain integration tests only on schedule/release/dispatch' directly and clearly summarizes the main change in the changeset. The title accurately reflects the primary modification—configuring CI workflows to run integration tests only under specific conditions (schedule, release, or dispatch triggers). The title is concise, specific, and provides meaningful information about the change without being vague or misleading.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch v1-export-cron

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 581d12f and 8101d12.

📒 Files selected for processing (3)
  • .github/INTEGRATION_TESTS_ISSUE_TEMPLATE.md (1 hunks)
  • .github/workflows/forest.yml (3 hunks)
  • documentation/src/developer_documentation/release_checklist.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/INTEGRATION_TESTS_ISSUE_TEMPLATE.md
  • .github/workflows/forest.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: tests
  • GitHub Check: tests-release
  • GitHub Check: Build forest binaries on Linux AMD64
🔇 Additional comments (1)
documentation/src/developer_documentation/release_checklist.md (1)

25-25: Clear requirement addition aligned with CI workflow changes.

The new requirement to include the Release label fits naturally into the checklist and supports the PR objective of gating integration tests. The wording is direct and actionable.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d143ac5 and 581d12f.

📒 Files selected for processing (3)
  • .github/INTEGRATION_TESTS_ISSUE_TEMPLATE.md (1 hunks)
  • .github/workflows/forest.yml (3 hunks)
  • documentation/src/developer_documentation/release_checklist.md (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
🪛 actionlint (1.7.8)
.github/workflows/forest.yml

632-632: unexpected key "always_required" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"

(syntax-check)


653-653: unexpected key "only_release_schedule_manual" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"

(syntax-check)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: All lint checks
  • GitHub Check: tests-release
  • GitHub Check: tests
  • GitHub Check: Build forest binaries on Linux AMD64
🔇 Additional comments (5)
documentation/src/developer_documentation/release_checklist.md (1)

25-25: Clear process requirement.

The addition of the Release label requirement aligns well with the CI workflow changes and enforces a consistent release procedure.

.github/INTEGRATION_TESTS_ISSUE_TEMPLATE.md (1)

1-8: Appropriate issue template structure.

The template is well-structured and the use of {{ env.WORKFLOW_URL }} provides direct access to workflow logs for debugging failures. Ensure the WORKFLOW_URL environment variable is properly set before this template is used (verified in forest.yml at line 665).

.github/workflows/forest.yml (3)

6-7: Schedule trigger properly configured.

The cron schedule is correctly formatted and the comment clearly documents the execution time. This complements the conditional gating strategy.


273-274: Release label gating correctly implemented.

The conditional properly restricts the v1 export job to scheduled runs, manual dispatch, or PRs with the Release label. This aligns with the release checklist requirement.


662-673: Workflow URL and issue creation logic looks sound.

The WORKFLOW_URL setup is correct and provides actionable debugging information. The issue creation is appropriately gated to main-branch failures. However, this logic cannot execute until the critical YAML syntax issue (lines 631-656) is resolved.

Verify that this logic works correctly after fixing the critical YAML syntax issue.

@LesnyRumcajs LesnyRumcajs marked this pull request as draft November 3, 2025 10:28
@LesnyRumcajs LesnyRumcajs marked this pull request as ready for review November 3, 2025 14:22
@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Nov 3, 2025
Merged via the queue into main with commit 8b9d86e Nov 3, 2025
56 checks passed
@LesnyRumcajs LesnyRumcajs deleted the v1-export-cron branch November 3, 2025 16:55
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.

3 participants