-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #412, git error in CodeQL Analyze Action #413
Conversation
cab2521
to
15685d7
Compare
Since |
Adds new parameters to match updated cFS-CodeQL workflow interface introduced in nasa/cFS#413
.github/workflows/codeql-build.yml
Outdated
|
||
- name: Run tests | ||
run: ${{ inputs.tests }} | ||
# - name: Run tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArielSAdamsNASA, what is the use case for including tests in the CodeQL workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't run the unit tests here, where do they get run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note I just went through a significant effort to fix a large number of uninitialized variables being used in app unit tests, something that I'd expect analysis tools to catch early and avoid. I'm in favor of doing analysis on the unit tests, since the sort of warnings/errors that show up really should be addressed (earlier in the unit test development phase the better).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think as long as we build the tests, the analysis tools should catch problems with them. My question was whether it makes sense to run them here or in a different workflow like https://github.com/nasa/cFS/blob/main/.github/workflows/build-cfs.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy and concur! As long as they still get analyzed, and the tests get run somewhere in CI I'm happy!
7839272
to
9078af5
Compare
Adds new parameters to match updated cFS-CodeQL workflow interface introduced in nasa/cFS#413
1f2cf94
to
f281f71
Compare
- Add new parameters to match updated cFS-CodeQL workflow interface introduced in nasa/cFS#413 - Add file-exclusion checks to Action trigger so workflow doesn't run if only changes in commit or pull request are to documentation
7986c51
to
e3f8b77
Compare
Renames workflows to better describe what each one does. The CodeQL "reusable" workflow is meant to be used by other workflows. The CodeQL "Analysis" workflow calls the "reusable" CodeQL workflow to execute the static analysis runs. Co-authored-by: Ariel Adams <ArielSAdamsNASA@users.noreply.github.com>
b56c955
to
169cfdb
Compare
Fixes errors in CodeQL results uploads step. Update parameters in CodeQL "reusable" workflow. BREAKING Interface changes: - Renames callable workflow to `codeql-reusable.yml`, submodules will have to be updated - Adds required `component-path` input parameter - Repurpose tests input to be a boolean tied to "ENABLE_UNIT_TESTS" flag Internal changes: - Use git clone instead of checkout@v2 for the cFS-Bundle - Use symlink to map calling repo workspace to expected cFS Bundle directory location - Enable "code snippets" option to CodeQL Analyze action - Archives sarif files from analysis output - Removes code duplication by using a matrix build for security and coding standard analyses - Alphabetizes workflow inputs and order based on "required" flag
169cfdb
to
98e2ef8
Compare
Fix #412, git error in CodeQL Analyze Action
- Add new parameters to match updated cFS-CodeQL workflow interface introduced in nasa/cFS#413 - Add file-exclusion checks to Action trigger so workflow doesn't run if only changes in commit or pull request are to documentation
Checklist (Please check before submitting)
Describe the contribution
Fix #412
directory location
Testing performed
Called codeql analysis action in bundle repo
https://github.com/astrogeco/cFS/actions/runs/1791252476
Called fork implementation from cFE repo. See actions run in nasa/cFE#2035,
https://github.com/astrogeco/cFE/runs/5056853287?check_suite_focus=true
Expected behavior changes
System(s) tested on
Additional context
Will break current CodeQL implementation in cFS components.
Contributor Info - All information REQUIRED for consideration of pull request
Gerardo E. Cruz-Ortiz, NASA