analogdevicesinc / msdk

Software Development Kit for Analog Device's MAX-series microcontrollers
Apache License 2.0
60 stars 75 forks source link

PR Check Actions Not Operating as Intended #566

Closed jdk-maxim closed 1 year ago

jdk-maxim commented 1 year ago

For recent pull requests, I found that several of the required Check Actions are being skipped incorrectly.

Build_Examples

This fails on recent pull request, but passed on virtually identical one from last week. I noticed that last weeks just skipped running this test completely.

Looking through list of recent PRs, several appear to skip the actual build run, after "Check watch files" stage, due to "$RUN_TEST" never being set. For example: dtm sweep with better plotting MAX32570 cameraif dma bit index updates, tested it works

Guessing this section might fail due to case sensitivity:

  # Assume we want to actually run the workflow if no files changed
  for watch_file in $WATCH_FILES; do 
    if [[ "$CHANGE_FILES" == *"$watch_file" ]]; then
      RUN_TEST=1
    fi
  done

Lint Code Base

This passed but didn't actually test anything. Log shows:

 fatal: ambiguous argument 'master...6732539c3fa07fc82c54e01ccc3976fc862b8405': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
2023-04-25 14:45:52 [WARN]   No files were found in the GITHUB_WORKSPACE:[/github/workspace] to lint!

But this did not get bubbled up to to the top for a Red Failing checking. Maybe this failed since we migrated from master->main?

Verify Register and SVD Files

This doesn't get run even though the PR contains changes to SVD and some "*_regs.h" files.

sihyung-maxim commented 1 year ago
  1. There have been some changes to the Build_Examples and BLE_Examples workflows recently which have caused those workflows to fail occasionally in the past few weeks. I believe the changes have been (mostly) finalized.

  2. I've updated the linter's default branch to main instead of master.

  3. Something in the Verify_Register_SVD's workspace is preventing git diff from working. I'm taking a look at it now.

sihyung-maxim commented 1 year ago

The Verify_Register_SVD workflow checkouts the msdk twice if the source branch of the PR is from a forked branch. This resulted in the git diff returning nothing even if there are file changes. I updated the workflow to make sure it only checks out the branch once.

All the changes were merged with PR: #565.

Jake-Carter commented 1 year ago

Thanks @sihyung-maxim

In #545 I fixed some issues but didn't modify the bash script that checks for watch files that Jeremy pointed out above.

Keeping an eye on things... We may need to reopen this to reimplement the check if it continues to fail intermittently after your latest fixes

Jake-Carter commented 1 year ago

Register file changes in #534 did not trigger build_examples

jdk-maxim commented 1 year ago

Did some testing with the watch file test. Think it only checking the last file in the "$CHANGE_FILES" list instead of iterating though the entire list. This seems to work:

for watch_file in $WATCH_FILES; do 
    for change_file in $CHANGE_FILES; do 
        if [[ "${change_file,,}" == *"${watch_file,,}" ]]; then
            echo "MATCH CI Iterate. Watch type: $watch_file, File: $change_file"
            RUN_TEST=1
        fi
    done
done  

Iterates through all strings in CHANGE_FILES and WATCH_FILES.

uses the ${$VAR,,} to force case to lower for comparison.

sihyung-maxim commented 1 year ago

Whoops, did not know merging a PR with a linked issue would automatically close the issue. My bad @Jake