[PR #415] [MERGED] Fixes include when using matrix and strategy build. #1442

New issue

Closed

opened 2026-03-01 21:51:11 +03:00 by kerem · 0 comments

kerem commented

2026-03-01 21:51:11 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/nektos/act/pull/415
Author: @SteffenSeckler
Created: 11/11/2020
Status: ✅ Merged
Merged: 12/8/2020
Merged by: @cplee

Base: master ← Head: feature/fixes_include

📝 Commits (7)

2a33489 fixes include directive of strategy build.
97b9970 Adds test for include and exclude in matrix builds.
7159584 ubuntu-16.04 instead of 20.04
9f26d04 Adds more platforms for runner_test
082da61 correct printing for unsupported platform
467f958 Merge remote-tracking branch 'source/master' into feature/fixes_include
61626e3 fix merge

📊 Changes

4 files changed (+42 additions, -10 deletions)

View changed files

📝 pkg/model/workflow.go (+6 -9)
📝 pkg/runner/run_context.go (+4 -1)
📝 pkg/runner/runner_test.go (+1 -0)
➕ pkg/runner/testdata/matrix-include-exclude/push.yml (+31 -0)

📄 Description

The include of the matrix strategy was incorrectly handled.
This simply appends the provided includes to the matrix product, s.t. this works (taken from the official doc)

runs-on: ${{ matrix.os }}
strategy:
  matrix:
    os: [macos-latest, windows-latest, ubuntu-18.04]
    node: [4, 6, 8, 10]
    include:
      # includes a new variable of npm with a value of 2
      # for the matrix leg matching the os and version
      - os: windows-latest
        node: 4
        npm: 2

Also replaces the comparison of the commonKeysMatch() with a reflect.DeepEqual to be able to compare maps.
Potentially the entire loop could be replaced that way`

PS:
I do not know how testing works in this project, so I cannot easily add one. But I have seen, that neither include nor exclude appears to be tested.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/nektos/act/pull/415 **Author:** [@SteffenSeckler](https://github.com/SteffenSeckler) **Created:** 11/11/2020 **Status:** ✅ Merged **Merged:** 12/8/2020 **Merged by:** [@cplee](https://github.com/cplee) **Base:** `master` ← **Head:** `feature/fixes_include` --- ### 📝 Commits (7) - [`2a33489`](https://github.com/nektos/act/commit/2a334890f56e8bfb72a7aea3fe8be3e0315f4df7) fixes include directive of strategy build. - [`97b9970`](https://github.com/nektos/act/commit/97b99705fe9a4bd33ceb2c08637eae69f3730e2c) Adds test for include and exclude in matrix builds. - [`7159584`](https://github.com/nektos/act/commit/7159584984d1ca8e6cff1e2f2621ec7b6d462c35) ubuntu-16.04 instead of 20.04 - [`9f26d04`](https://github.com/nektos/act/commit/9f26d04802217096fe7aa31b75559aeadcfb681d) Adds more platforms for runner_test - [`082da61`](https://github.com/nektos/act/commit/082da613084d81b1a751082b09914c1d919d84b4) correct printing for unsupported platform - [`467f958`](https://github.com/nektos/act/commit/467f95816e0121bfb835037ae483fb51ce15933d) Merge remote-tracking branch 'source/master' into feature/fixes_include - [`61626e3`](https://github.com/nektos/act/commit/61626e338966f8acc5ad107eee702aa52f3e9bc1) fix merge ### 📊 Changes **4 files changed** (+42 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `pkg/model/workflow.go` (+6 -9) 📝 `pkg/runner/run_context.go` (+4 -1) 📝 `pkg/runner/runner_test.go` (+1 -0) ➕ `pkg/runner/testdata/matrix-include-exclude/push.yml` (+31 -0) </details> ### 📄 Description The `include` of the matrix strategy was incorrectly handled. This simply appends the provided includes to the matrix product, s.t. this works (taken from [the official doc](https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#example-including-additional-values-into-combinations)) ```yaml runs-on: ${{ matrix.os }} strategy: matrix: os: [macos-latest, windows-latest, ubuntu-18.04] node: [4, 6, 8, 10] include: # includes a new variable of npm with a value of 2 # for the matrix leg matching the os and version - os: windows-latest node: 4 npm: 2 ``` Also replaces the comparison of the `commonKeysMatch()` with a `reflect.DeepEqual` to be able to compare maps. Potentially the entire loop could be replaced that way` PS: I do not know how testing works in this project, so I cannot easily add one. But I have seen, that neither `include` nor `exclude` appears to be tested. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-03-01 21:51:11 +03:00

closed this issue
added the
pull-request
label

kerem referenced this issue

2026-03-01 21:53:14 +03:00

[PR #1391] [MERGED] feat: GITHUB_STATE and GITHUB_OUTPUT file commands #1919

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/act#1442

No description provided.

Rows
Columns