Skip to content

744/5: workaround for postman#747

Open
al-niessner wants to merge 3 commits intodevelopfrom
issue_745
Open

744/5: workaround for postman#747
al-niessner wants to merge 3 commits intodevelopfrom
issue_745

Conversation

@al-niessner
Copy link
Contributor

🗒️ Summary

Pulled the testing from actions to local machine but verify that it has been run and successful in actions. Cheater and inexpensive off loading...

🤖 AI Assistance Disclosure

  • No AI assistance used
  • AI used for light assistance (e.g., suggestions, refactoring, documentation help, minor edits)
  • AI used for moderate content generation (AI generated some code or logic, but the developer authored or heavily revised the majority)
  • AI generated substantial portions of this code

Estimated % of code influenced by AI: ___ %

⚙️ Test Data and/or Report

All tests should pass

♻️ Related Issues

Closes #744
Closes #745

🤓 Reviewer Checklist

Reviewers: Please verify the following before approving this pull request.

Documentation and PR Content

  • Documentation: README, Wiki, or inline documentation (Sphinx, Javadoc, Docstrings) have been updated to reflect these changes.
  • Issue Traceability: The PR is linked to a valid GitHub Issue
  • PR Title: The PR title is "user-friendly" clearly identifying what is being fixed or the new feature being added, that if you saw it in the Release Notes for a tool, you would be able to get the gist of what was done.

Security & Quality

  • SonarCloud: Confirmed no new High or Critical security findings.
  • Secrets Detection: Verified that the Secrets Detection scan passed and no sensitive information (keys, tokens, PII) is exposed.
  • Code Quality: Code follows organization style guidelines and best practices for the specific language (e.g., PEP 8, Google Java Style).

Testing & Validation

  • Test Accuracy: Verified that test data is accurate, representative of real-world PDS4 scenarios, and sufficient for the logic being tested.
  • Coverage: Automated tests cover new logic and edge cases.
  • Local Verification: (If applicable) Successfully built and ran the changes in a local or staging environment.

Maintenance

  • Backward Compatibility: Confirmed that these changes do not break existing downstream dependencies or API contracts (or that breaking changes are clearly documented).

@al-niessner al-niessner self-assigned this Mar 25, 2026
@al-niessner al-niessner requested a review from a team as a code owner March 25, 2026 21:09
@al-niessner
Copy link
Contributor Author

@jordanpadams @tloubrieu-jpl @nutjob4life

Y'all are gonna hate me for this. Wrote a script to do the work locally. It saves a JSON file with information to tell github actions if there was success or not. Solves the small action resources even as tests grow. Keeps enforcing testing. Yes, can fake it, but can fake anything. For instance, postman was returning success despite having 12 assertion failures . Turns out the return status is not all that reliable. Have to do some log parsing. Minimal today, but can improve as it evolves.

Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

Hah! Wow, this is … actually pretty nifty, @al-niessner. But I think it could use some docs as well, since the CI itself is now "part of the official record" (git history) with the last_integration_test.json file, and there are some new motions developers have to go through:

  • README update: explain what last_integration_test.json, when it must be updated, how, what GitHub Actions is actually checking
  • Maybe this repo should have its own .github/pull_request_template.md with an extra checkbox reminding people to do that?

# The EXIT pseudo-signal covers normal exits, errors, and interruptions (Ctrl+C)
trap 'rm -rf "$tdir"' EXIT
cd "$tdir" || exit 1
git clone --quiet https://github.com/NASA-PDS/registry.git
Copy link
Member

Choose a reason for hiding this comment

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

Is this okay? It looks like we're just cloning HEAD and so makes this repository dependent on whatever the state of registry happens to be at the time.

I'm okay with that—just want to double-check.

@al-niessner
Copy link
Contributor Author

Hah! Wow, this is … actually pretty nifty, @al-niessner. But I think it could use some docs as well, since the CI itself is now "part of the official record" (git history) with the last_integration_test.json file, and there are some new motions developers have to go through:

  • README update: explain what last_integration_test.json, when it must be updated, how, what GitHub Actions is actually checking
  • Maybe this repo should have its own .github/pull_request_template.md with an extra checkbox reminding people to do that?

The JSON is regenerated when you run .github/workflows/integration_tests.sh. No manual editing of the JSON file. Question is, do you add a commit hook (no because there is a fixed 8 minute wait in there for reasons I cannot fathom) or something similar. Maybe the push. However, until working robustly and quickly, just do it manually. May need to move the script to make it more obvious - I would suggest a script to call this script because this script pretends it knows where it is.

Oh docs. Sure...

@nutjob4life
Copy link
Member

No manual editing of the JSON file

Ohh I missed that. Sorry.

Docs for the win!

Need to add similar content to readme.md but it is most important here because the developer will be trying to understand why github actions is giving a red X to find the solution in the github actions output.
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants