Skip to content

Feature: Add test step to CI workflow.#3459

Open
twoGiants wants to merge 1 commit intoknative:mainfrom
twoGiants:issue-781-ci-test-step
Open

Feature: Add test step to CI workflow.#3459
twoGiants wants to merge 1 commit intoknative:mainfrom
twoGiants:issue-781-ci-test-step

Conversation

@twoGiants
Copy link
Contributor

Changes

  • 🎁 Add --test-step flag to func config ci, enabled by default, disable with --test-step=false
  • 🎁 Generate a "Run tests" step in the GitHub Actions workflow that executes before deployment
  • 🎁 Select the test command based on function runtime: go test ./... for Go, npm test for Node.js, python -m pytest for Python
  • 🎁 Skip the test step with a warning for unsupported runtimes

/kind enhancement

Relates to #3256

Release Note

New `--test-step` flag for `func config ci` adds a runtime-appropriate test step to the CI workflow before deployment.

Docs


@knative-prow knative-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 27, 2026
@twoGiants
Copy link
Contributor Author

/cc @matejvasek @lkingland @gauron99

Next one is ready for review! 😸 👍

@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 71.81818% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.71%. Comparing base (fbd2f4f) to head (910e14a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/ci/workflow.go 38.23% 18 Missing and 3 partials ⚠️
cmd/ci/config.go 63.63% 8 Missing ⚠️
cmd/ci/common.go 0.00% 0 Missing and 1 partial ⚠️
cmd/ci/printer.go 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3459      +/-   ##
==========================================
- Coverage   54.71%   54.71%   -0.01%     
==========================================
  Files         181      181              
  Lines       20394    20427      +33     
==========================================
+ Hits        11158    11176      +18     
- Misses       8059     8075      +16     
+ Partials     1177     1176       -1     
Flag Coverage Δ
e2e 38.25% <0.00%> (-0.08%) ⬇️
e2e go 32.49% <0.00%> (-0.07%) ⬇️
e2e node 28.30% <0.00%> (-0.04%) ⬇️
e2e python 31.90% <0.00%> (-0.07%) ⬇️
e2e quarkus 28.43% <0.00%> (-0.08%) ⬇️
e2e rust 27.85% <0.00%> (-0.06%) ⬇️
e2e springboot 26.34% <0.00%> (-0.06%) ⬇️
e2e typescript 28.42% <0.00%> (-0.06%) ⬇️
integration 17.43% <0.00%> (-0.04%) ⬇️
unit macos-14 42.63% <71.13%> (+<0.01%) ⬆️
unit macos-latest 42.63% <71.13%> (+<0.01%) ⬆️
unit ubuntu-24.04-arm 42.88% <71.81%> (+<0.01%) ⬆️
unit ubuntu-latest 43.51% <71.13%> (+<0.01%) ⬆️
unit windows-latest 42.65% <71.13%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@gauron99 gauron99 left a comment

Choose a reason for hiding this comment

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

nice! just few questions from me

case "go":
testStep.withRun("go test ./...")
case "node":
testStep.withRun("npm test")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need to call npm install first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do, thx!

case "node":
testStep.withRun("npm test")
case "python":
testStep.withRun("python -m pytest")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need to call pip instal… first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do, thx!

@twoGiants twoGiants force-pushed the issue-781-ci-test-step branch from b04f22b to 93278b3 Compare March 2, 2026 08:37
@twoGiants twoGiants requested review from gauron99 and matejvasek March 2, 2026 08:37
@gauron99
Copy link
Contributor

gauron99 commented Mar 2, 2026

/lgtm
/approve
/hold if matej has any more changes

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 2, 2026
@knative-prow knative-prow bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 2, 2026
Copy link
Member

@lkingland lkingland left a comment

Choose a reason for hiding this comment

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

Solid work, very clean!

/lgtm
/approve
/hold for anyone else that might want to chime in more

{
name: "nodejs runtime adds npm test step",
runtime: "node",
expectedRun: "npm i && npm test",
Copy link
Member

Choose a reason for hiding this comment

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

I seem to remember something about an npm ci command specific for this use-case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes of course! 🙈

Updated it!

@knative-prow
Copy link

knative-prow bot commented Mar 2, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauron99, lkingland, twoGiants

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@twoGiants twoGiants force-pushed the issue-781-ci-test-step branch from 93278b3 to 910e14a Compare March 2, 2026 13:06
@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2026
@matejvasek
Copy link
Contributor

Doc docs/reference/func_config_ci.md is not regenerated.

@twoGiants
Copy link
Contributor Author

Doc docs/reference/func_config_ci.md is not regenerated.

Running ./hack/update-codegen.sh did not produce any changes. But the CI should've caught this anyway, or? @matejvasek

@matejvasek
Copy link
Contributor

matejvasek commented Mar 2, 2026

Doc docs/reference/func_config_ci.md is not regenerated.

Running ./hack/update-codegen.sh did not produce any changes. But the CI should've caught this anyway, or? @matejvasek

The command (and therefore docs gen) is behind feature flag export FUNC_ENABLE_CI_CONFIG=true.

diff --git a/docs/reference/func_config_ci.md b/docs/reference/func_config_ci.md
index 1c87a5fbd..74cfd1748 100644
--- a/docs/reference/func_config_ci.md
+++ b/docs/reference/func_config_ci.md
@@ -9,17 +9,21 @@ func config ci
 ### Options
 
 ```
-      --branch string                             Use a custom branch name in the workflow (default "main")
+      --branch string                             Use a custom branch name in the workflow
+      --force                                     Use to overwrite an existing GitHub workflow
   -h, --help                                      help for ci
       --kubeconfig-secret-name string             Use a custom secret name in the workflow, e.g. secret.YOUR_CUSTOM_KUBECONFIG (default "KUBECONFIG")
   -p, --path string                               Path to the function.  Default is current directory ($FUNC_PATH)
+      --platform string                           Pick a CI/CD platform for which a manifest will be generated. Currently only GitHub is supported. (default "github")
+      --registry-login                            Add a registry login step in the github workflow (default true)
       --registry-login-url-variable-name string   Use a custom registry login url variable name in the workflow, e.g. vars.YOUR_REGISTRY_LOGIN_URL (default "REGISTRY_LOGIN_URL")
       --registry-pass-secret-name string          Use a custom registry pass secret name in the workflow, e.g. secret.YOUR_REGISTRY_PASSWORD (default "REGISTRY_PASSWORD")
       --registry-url-variable-name string         Use a custom registry url variable name in the workflow, e.g. vars.YOUR_REGISTRY_URL (default "REGISTRY_URL")
       --registry-user-variable-name string        Use a custom registry user variable name in the workflow, e.g. vars.YOUR_REGISTRY_USER (default "REGISTRY_USERNAME")
       --remote                                    Build the function on a Tekton-enabled cluster
       --self-hosted-runner                        Use a 'self-hosted' runner instead of the default 'ubuntu-latest' for local runner execution
-      --use-registry-login                        Add a registry login step in the github workflow (default true)
+      --test-step                                 Add a language-specific test step (supported: go, node, python) (default true)
+  -v, --verbose                                   Print verbose logs ($FUNC_VERBOSE)
       --workflow-name string                      Use a custom workflow name (default "Func Deploy")
 ```
 

@twoGiants twoGiants force-pushed the issue-781-ci-test-step branch from 910e14a to b82164a Compare March 3, 2026 09:12
@gauron99
Copy link
Contributor

gauron99 commented Mar 3, 2026

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2026
The generated GitHub workflow now includes a "Run tests"
step that runs before deployment. The test command is
selected based on the function runtime: `go test ./...` for
Go, `npm ci && npm test` for Node.js and Typescript, for Python
`pip install . && python -m pytest` and `./mvnw test` for
Quarkus. Unsupported runtimes skip the step with a warning.
The step can be disabled via --test-step=false.

Issue 3256

Signed-off-by: Stanislav Jakuschevskij <sjakusch@redhat.com>
@twoGiants twoGiants force-pushed the issue-781-ci-test-step branch from b82164a to 2ed8b2b Compare March 3, 2026 09:20
@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2026
@knative-prow
Copy link

knative-prow bot commented Mar 3, 2026

New changes are detected. LGTM label has been removed.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/enhancement size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants