Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/ext-build-examples-repo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,11 @@ jobs:
target_repository: nasa/fprime-examples
build_location: FlightExamples
run_unit_tests: true
run_integration_tests: true
integration_gds_working_directory: FlightExamples/ExamplesDeployment
integration_test_working_directory: FlightExamples
integration_binary: ../build-artifacts/*/ExamplesDeployment/bin/ExamplesDeployment
integration_gds_args: "--dictionary ../build-artifacts/*/ExamplesDeployment/dict/ExamplesDeploymentTopologyDictionary.json"
integration_pytest_args: "DataProduct/test/int ManagerWorker/Subtopology/test/int --dictionary build-artifacts/*/ExamplesDeployment/dict/ExamplesDeploymentTopologyDictionary.json --no-zmq"
Comment on lines +44 to +46

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Shell glob patterns in binary and dictionary paths could match multiple entries

The integration_binary path (ext-build-examples-repo.yml:44) uses ../build-artifacts/*/ExamplesDeployment/bin/ExamplesDeployment with a shell glob *. The chmod +x step at reusable-project-builder.yml:228 runs this unquoted in bash, so the glob will expand. If build-artifacts/ contains exactly one platform directory (e.g. Linux), this works correctly. If multiple platform directories exist (unlikely in CI with a single target_platform), the chmod would apply to all matches (harmless), but the binary input passed to the run-integration-tests action would be a literal unexpanded string since GitHub Actions inputs are strings, not shell-expanded. The action would need to handle glob expansion internally for this to work.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The globs are expanded by the shell inside nasa/fprime-actions/run-integration-tests, which uses $BINARY_PATH/$GDS_ARGS unquoted when launching the FSW binary and fprime-gds. In CI there's exactly one platform directory under build-artifacts/ (the single target_platform), so each glob resolves to a single path. Confirmed against the live run: the chmod +x step and the FSW launch both resolved the binary correctly. The * is used deliberately so the workflow is platform-agnostic (e.g. Linux vs Darwin) rather than hard-coding the artifact directory name.

target_ref: ${{ needs.get-branch.outputs.target-branch }}
fprime_location: ./FlightExamples/lib/fprime
103 changes: 103 additions & 0 deletions .github/workflows/reusable-project-builder.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,41 @@ on:
required: false
type: boolean
default: true
run_integration_tests:
description: "Run an additional job to build the deployment and run integration tests against it."
required: false
type: boolean
default: false
integration_gds_working_directory:
description: "Directory to start fprime-gds from (relative to repo root). Required when run_integration_tests is true."
required: false
type: string
default: ""
integration_test_working_directory:
description: "Directory to run pytest from (relative to repo root). Required when run_integration_tests is true."
required: false
type: string
default: ""
integration_binary:
description: "Path to the FSW binary to run (relative to integration_gds_working_directory). Required when run_integration_tests is true."
required: false
type: string
default: ""
integration_binary_args:
description: "Arguments passed to the FSW binary."
required: false
type: string
default: "-a 127.0.0.1 -p 50000"
Comment on lines +42 to +46

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Default binary-args may override the action's own default behavior

The integration_binary_args input defaults to -a 127.0.0.1 -p 50000 and is always passed through to the run-integration-tests action as binary-args. In ref.yml:110-118, the same action is called without any binary-args parameter, relying on the action's own default. If the action's default differs from -a 127.0.0.1 -p 50000, callers of this reusable workflow that don't explicitly set integration_binary_args will get different behavior than the Ref deployment's integration tests. This should be verified against the nasa/fprime-actions/run-integration-tests action's action.yml.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Verified against run-integration-tests/action.yml: its binary-args default is -a 127.0.0.1 -p 50000 — identical to this input's default, so there's no behavioral difference from ref.yml (which relies on that same action default). I kept an explicit default here rather than an empty string because the reusable workflow always forwards binary-args, and forwarding "" would override (blank out) the action's default rather than fall back to it.

integration_gds_args:
description: "Extra arguments passed to fprime-gds (e.g. --dictionary path)."
required: false
type: string
default: ""
integration_pytest_args:
description: "Extra arguments passed to pytest (e.g. test paths, --dictionary path)."
required: false
type: string
default: ""
fprime_location:
description: "Relative path from the external project root to its F´ submodule"
required: false
Expand Down Expand Up @@ -139,3 +174,71 @@ jobs:
fprime-util check -j8 ${VERBOSE_FLAG} ${TARGET_PLATFORM} --pass-through --output-on-failure
shell: bash

runIntegration:
if: ${{ inputs.run_integration_tests }}
runs-on: ${{ inputs.runs_on }}
name: "Integration Tests"
env:
FPRIME_LOCATION: ${{ inputs.fprime_location }}
TARGET_PLATFORM: ${{ inputs.target_platform }}
steps:
- name: "Checkout target repository"
uses: actions/checkout@v4
with:
submodules: recursive
repository: ${{ inputs.target_repository }}
ref: ${{ inputs.target_ref }}
- name: "Overlay current F´ revision"
uses: actions/checkout@v4
with:
submodules: true
path: ${{ inputs.fprime_location }}
# Install the target project's requirements so its GDS plugins (e.g. custom framing)
# are available to fprime-gds; fall back to the F´ submodule requirements otherwise.
- name: "Install requirements.txt"
run: |
# Modern build tooling is required so local source packages (e.g. the
# project's GDS plugins) build with correct metadata; the runner's
# system pip/setuptools can otherwise produce a broken "UNKNOWN" wheel
# with no entry points, which silently drops custom framing plugins.
pip3 install --upgrade pip setuptools wheel
if [ -f "${{ inputs.build_location }}/requirements.txt" ]; then
(cd "${{ inputs.build_location }}" && pip3 install -r requirements.txt)
else
pip3 install -r ${FPRIME_LOCATION}/requirements.txt
fi
shell: bash
Comment on lines +198 to +210

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Integration job uses different requirements.txt installation logic than build and UT jobs

The integration job (reusable-project-builder.yml:199-204) has a unique requirements installation strategy: it first checks for ${{ inputs.build_location }}/requirements.txt and only falls back to the F´ submodule requirements if that file doesn't exist. The comment at line 196-197 explains this is to pick up GDS plugins (e.g. custom framing). The build job (line 99-100) and runUT job (line 143-144) always install from ${FPRIME_LOCATION}/requirements.txt. This behavioral difference is intentional but means the integration job could have a different Python environment than the build job — if the project-level requirements.txt doesn't include everything from the F´ requirements.txt, the integration build step could behave differently.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Intentional. The integration job needs the target project's GDS-side dependencies — specifically its custom fprime-gds plugins (e.g. the ExamplesDeployment decaf framing plugin in GdsExamples/gds-plugins) — which are only declared in the project's own requirements.txt, not in F´'s. The build/UT jobs only need FSW build deps, so they install F´'s requirements. When the project has no requirements.txt we fall back to F´'s requirements, preserving prior behavior for callers that don't need GDS plugins.

- name: "Generate build cache"
working-directory: ${{ inputs.build_location }}
run: |
if [ "${RUNNER_DEBUG:-0}" = "1" ]; then
VERBOSE_FLAG=--verbose
else
VERBOSE_FLAG=
fi
fprime-util generate ${VERBOSE_FLAG} ${TARGET_PLATFORM}
shell: bash
- name: "Build"
working-directory: ${{ inputs.build_location }}
run: |
if [ "${RUNNER_DEBUG:-0}" = "1" ]; then
VERBOSE_FLAG=--verbose
else
VERBOSE_FLAG=
fi
fprime-util build -j8 ${VERBOSE_FLAG} ${TARGET_PLATFORM}
shell: bash
- name: "Make FSW binary executable"
working-directory: ${{ inputs.integration_gds_working_directory }}
run: chmod +x ${{ inputs.integration_binary }}
shell: bash
- name: "Integration Tests"
uses: nasa/fprime-actions/run-integration-tests@devel
with:
gds-working-directory: ${{ inputs.integration_gds_working_directory }}
test-working-directory: ${{ inputs.integration_test_working_directory }}
binary: ${{ inputs.integration_binary }}
binary-args: ${{ inputs.integration_binary_args }}
gds-args: ${{ inputs.integration_gds_args }}
pytest-args: ${{ inputs.integration_pytest_args }}

Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Loading