chore: fix the manual cross platform test run#9252
Conversation
WalkthroughThis update enhances the cross-platform test workflows by introducing explicit input validation, additional debugging steps, and a new debug job. It ensures that required inputs are present, prints detailed parameter and artifact information, and adjusts job dependencies and input handling for clarity and traceability. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant BuildJob as build-if-needed
participant DebugJob as debug-parameters
participant TestJob as test-wheel-installation
Workflow->>BuildJob: Start build-if-needed
BuildJob->>BuildJob: Print artifact names & parameters (debug step)
BuildJob-->>Workflow: Outputs artifact names
Workflow->>DebugJob: Start debug-parameters (needs: build-if-needed)
DebugJob->>DebugJob: Print resolved parameters & build outputs
Workflow->>TestJob: Start test-wheel-installation (needs: build-if-needed, debug-parameters)
TestJob->>TestJob: Validate & print input parameters (shared workflow)
TestJob->>TestJob: Run tests with validated inputs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
SonicDMG
temporarily deployed
to
fix_manual_cp_test - langflow-manual-install PR #9252
Render
Destroyed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.github/workflows/cross-platform-test-shared.yml (2)
82-97: Add basic Bash safety flags to the debug stepRunning the debug script with
set -euo pipefail(or at leastset -e) will make the step fail fast on unexpected errors (e.g. missing variables, failed subshells) instead of silently continuing. It is free insurance and keeps the workflow behaviour deterministic.- - name: Debug workflow inputs - run: | + - name: Debug workflow inputs + run: | + set -euo pipefail echo "Shared workflow received inputs:"
99-109: Parameter validation can be moved to theif:guard and avoid an extra shell exitYou already know, at eval time, whether either name is missing. Using the expression language lets GitHub mark the whole job as
skippedinstead of launching a Bash process only toexit 1, saving one runner minute and keeping logs cleaner.# replace the existing step with a single-line noop that never runs - name: Validate required parameters for wheel installation if: inputs.install-method == 'wheel' && (inputs.base-artifact-name == '' || inputs.main-artifact-name == '') run: | echo "❌ base-artifact-name and main-artifact-name are required when install-method is 'wheel'" exit 1 shell: bash.github/workflows/cross-platform-test.yml (3)
84-92: Avoid hard-coding artifact names in the debug logThe step prints fixed strings (
adhoc-dist-base/adhoc-dist-main) instead of the actual output variables, which will mislead readers if the names ever change. Emit the resolved names directly fromsteps.set-names.outputs(or${{ needs.build-if-needed.outputs.* }}) to keep the log truthful.- echo " base-artifact-name: adhoc-dist-base" - echo " main-artifact-name: adhoc-dist-main" + echo " base-artifact-name: ${{ needs.build-if-needed.outputs.base-artifact-name }}" + echo " main-artifact-name: ${{ needs.build-if-needed.outputs.main-artifact-name }}"
93-111: Expressions can be simplified for readability
inputs.base-artifact-name != '' && inputs.base-artifact-nameis redundant—an empty string is already “falsey”. Dropping the comparison shortens the expression without changing semantics:${{ inputs.base-artifact-name || needs.build-if-needed.outputs.base-artifact-name || 'adhoc-dist-base' }}Same for
main-artifact-nameandtest-timeout. Less punctuation → easier maintenance.
119-121: Coalesce expressions the same way in thewith:blockAfter simplifying as above you can shorten the
with:section:- test-timeout: ${{ inputs.test-timeout || 5 }} - base-artifact-name: ${{ inputs.base-artifact-name != '' && inputs.base-artifact-name || needs.build-if-needed.outputs.base-artifact-name || 'adhoc-dist-base' }} - main-artifact-name: ${{ inputs.main-artifact-name != '' && inputs.main-artifact-name || needs.build-if-needed.outputs.main-artifact-name || 'adhoc-dist-main' }} + test-timeout: ${{ inputs.test-timeout || 5 }} + base-artifact-name: ${{ inputs.base-artifact-name || needs.build-if-needed.outputs.base-artifact-name || 'adhoc-dist-base' }} + main-artifact-name: ${{ inputs.main-artifact-name || needs.build-if-needed.outputs.main-artifact-name || 'adhoc-dist-main' }}Functionally identical, a little easier on future readers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/cross-platform-test-shared.yml(1 hunks).github/workflows/cross-platform-test.yml(1 hunks)
ogabrielluiz
changed the title
2 participants
An attempt to fix the manual cross-platform install test. Formerly the build worked but the test didn't initiate.
Summary by CodeRabbit