Skip to content

Conversation

dareste
Copy link
Collaborator

@dareste dareste commented Oct 13, 2025

Proposed changes

The integration test now verifies that the generated supportpkg contains a valid manifest.json file.

Tests include:

  • File existence.
  • JSON syntax validation.
  • Presence of the expected keys.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING guide
  • I have proven my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have ensured the README is up to date
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch on my own fork

@dareste dareste requested review from Copilot and mrajagopal October 13, 2025 13:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Enhances the integration test by adding manifest validation to ensure generated supportpkg files contain valid manifest.json with proper structure and required fields.

  • Adds a manifest validation step to the integration test workflow
  • Creates a comprehensive validation script that checks file existence, JSON syntax, and required keys
  • Implements detailed error reporting with specific exit codes for different failure scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/integration-test.yml Adds manifest validation step to integration test workflow
.github/scripts/verify-manifest.sh New script that validates manifest.json structure and content

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

dareste and others added 2 commits October 13, 2025 15:20
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Collaborator

@mrajagopal mrajagopal left a comment

Choose a reason for hiding this comment

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

Have we considered tallying the files with that captured in the manifest.json?

I have used the below set of commands to confirm the files in the tarball are in the manifest and vice versa:

find ./test -type f | sed 's/^\.\/test//' > find-output.txt  
dir=test; jq -r '.commands[].output' ./${dir}/manifest.json > manifest-output.txt
grep -vxFf find-output.txt manifest-output.txt
grep -vxFf manifest-output.txt find-output.txt
  • The first grep finds lines in manifest-output.txt that are NOT present in find-output.txt, this should yield zero files.

  • The second grep finds lines in find-output.txt that are not present in manifest-output.txt which should yield manifest.json and supportpkg.log only.

There is probably a better way, but a start seeing that I had already invested time in developing this using shell commands.

@dareste dareste requested a review from mrajagopal October 14, 2025 09:58
Copy link
Collaborator

@mrajagopal mrajagopal left a comment

Choose a reason for hiding this comment

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

All good.

@mrajagopal mrajagopal merged commit 903a6e8 into main Oct 15, 2025
6 checks passed
@mrajagopal mrajagopal deleted the areste-integration-test-update branch October 15, 2025 22:20
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.

2 participants