Skip to content
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

chore(performance): Adding breakpoint tests #1793

Merged
merged 24 commits into from
Feb 4, 2025
Merged

Conversation

dagfinno
Copy link
Collaborator

@dagfinno dagfinno commented Feb 4, 2025

Description

This PR adds breakpoint tests, where you give target number of vus, duration of test and option to abort the test of failures.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)

Documentation

  • Documentation is updated (either in docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)

@dagfinno dagfinno requested review from a team as code owners February 4, 2025 10:18
Copy link
Contributor

coderabbitai bot commented Feb 4, 2025

Warning

Rate limit exceeded

@dagfinno has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 33 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between d9e6582 and 595e3c8.

📒 Files selected for processing (15)
  • .azure/applications/graphql/main.bicep (1 hunks)
  • .azure/applications/service/main.bicep (1 hunks)
  • .azure/applications/sync-resource-policy-information-job/main.bicep (1 hunks)
  • .azure/applications/sync-subject-resource-mappings-job/main.bicep (1 hunks)
  • .azure/applications/web-api-eu/main.bicep (1 hunks)
  • .azure/applications/web-api-migration-job/main.bicep (1 hunks)
  • .azure/applications/web-api-so/main.bicep (1 hunks)
  • .azure/infrastructure/main.bicep (1 hunks)
  • .azure/modules/appConfiguration/addReaderRoles.bicep (1 hunks)
  • .azure/modules/appConfiguration/create.bicep (1 hunks)
  • .azure/modules/appConfiguration/upsertKeyValue.bicep (1 hunks)
  • .azure/modules/containerAppEnv/main.bicep (1 hunks)
  • .azure/modules/keyvault/copySecrets.bicep (1 hunks)
  • .azure/modules/privateDnsZone/main.bicep (2 hunks)
  • .azure/modules/redis/main.bicep (1 hunks)
📝 Walkthrough

Walkthrough

This pull request introduces two new GitHub Actions workflows for triggering and executing k6 breakpoint performance tests with customizable input parameters and secret handling. It adds several performance test scripts for both end-user and service owner functionalities, including tests for search, dialog creation, and transmissions. Additionally, a common helper function has been updated for export to allow reuse in other modules, and the Kubernetes test runner script has been enhanced with new command-line options to handle breakpoint tests.

Changes

Files Change Summary
.github/.../dispatch-k6-breakpoint.yml, .github/.../workflow-run-k6-breakpoint.yml Added two new GitHub Actions workflows to facilitate the execution of k6 breakpoint tests with customizable input parameters and secrets.
tests/k6/.../enduserSearchBreakpoint.js
tests/k6/.../createDialogBreakpoint.js
tests/k6/.../createTransmissionsBreakpoint.js
tests/k6/.../serviceOwnerSearchBreakpoint.js
Introduced new performance test scripts that export configuration objects, setup functions, and default test execution functions for various scenarios, including end-user search and service owner operations.
tests/k6/.../readTestdata.js Updated the endUsersPart function to be exported, enhancing modularity by allowing access from other modules.
tests/k6/.../run-test-in-k8s.sh Modified the script to support breakpoint tests by adding new command-line options (-b, --breakpoint and -a, --abort) and adjusting argument handling and naming conventions.
.github/.../dispatch-k6-performance.yml, .github/.../workflow-run-k6-performance.yml Added new input parameters (breakpoint and abortOnFail) to enhance control over performance test execution.

Possibly related PRs


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (17)
tests/k6/tests/scripts/run-test-in-k8s.sh (2)

24-26: Updated Help Text for Breakpoint and Abort Options
The help text now lists the new -b/--breakpoint and -a/--abort options. Consider clarifying that the -a/--abort flag requires an additional argument (for example, a boolean value like "true" or "false") so that users are not confused by its usage versus the simple switch used for -b/--breakpoint.


56-57: Initialization of New Flag Variables
The initialization of breakpoint=false and abort_on_fail=false is correct and ensures default values are set. It might be beneficial to later validate the value assigned via the -a/--abort flag to guarantee it matches the expected boolean format.

tests/k6/tests/serviceowner/performance/serviceOwnerSearchBreakpoint.js (3)

4-7: Consider increasing default test duration for more reliable results.

The default duration of '1m' might be too short to gather meaningful performance metrics, especially for ramping tests. Consider increasing the default to at least '5m' to ensure stable measurements.

-const stages_duration = (__ENV.stages_duration ?? '1m');
+const stages_duration = (__ENV.stages_duration ?? '5m');

19-24: Add threshold configurations for request count metrics.

The empty arrays for http_reqs metrics suggest missing threshold configurations. Consider adding appropriate thresholds to monitor request counts.

-        "http_reqs{name:get dialog activities}": [],
-        "http_reqs{name:get dialog activity}": [],
-        "http_reqs{name:get seenlogs}": [],
-        "http_reqs{name:get transmissions}": [],
-        "http_reqs{name:get transmission}": [],
-        "http_reqs{name:serviceowner search}": [],   
+        "http_reqs{name:get dialog activities}": [{ threshold: "rate>0", abortOnFail: false }],
+        "http_reqs{name:get dialog activity}": [{ threshold: "rate>0", abortOnFail: false }],
+        "http_reqs{name:get seenlogs}": [{ threshold: "rate>0", abortOnFail: false }],
+        "http_reqs{name:get transmissions}": [{ threshold: "rate>0", abortOnFail: false }],
+        "http_reqs{name:get transmission}": [{ threshold: "rate>0", abortOnFail: false }],
+        "http_reqs{name:serviceowner search}": [{ threshold: "rate>0", abortOnFail: false }],

41-44: Consider randomizing service owner selection for better test coverage.

The test always uses the first service owner (serviceOwners[0]). Consider randomizing the service owner selection to ensure better test coverage across different service owners.

-    serviceownerSearch(serviceOwners[0], endUsers[index], tag_name, traceCalls);
+    const randomServiceOwnerIndex = Math.floor(Math.random() * serviceOwners.length);
+    serviceownerSearch(serviceOwners[randomServiceOwnerIndex], endUsers[index], tag_name, traceCalls);
tests/k6/tests/serviceowner/performance/createDialogBreakpoint.js (4)

5-8: Consider adding input validation for environment variables.

While the default values are sensible, consider adding validation for:

  • stages_duration: Ensure it's a valid time format (e.g., '1m', '30s')
  • stages_target: Validate it's a positive integer
-const stages_duration = (__ENV.stages_duration ?? '1m');
-const stages_target = (__ENV.stages_target ?? '5');
+const stages_duration = validateDuration(__ENV.stages_duration, '1m');
+const stages_target = validateTarget(__ENV.stages_target, 5);

+function validateDuration(duration, defaultValue) {
+    if (!duration) return defaultValue;
+    if (!/^\d+[smh]$/.test(duration)) {
+        throw new Error('Invalid duration format. Use format: number + s/m/h (e.g., "30s", "1m", "1h")');
+    }
+    return duration;
+}

+function validateTarget(target, defaultValue) {
+    const num = parseInt(target || defaultValue);
+    if (isNaN(num) || num <= 0) {
+        throw new Error('Target must be a positive integer');
+    }
+    return num;
+}

14-14: Consider lowering the HTTP request duration threshold.

A 10-second threshold (10000ms) seems high for an API endpoint. Consider setting a more stringent threshold (e.g., 2000ms) to catch performance issues earlier.

-        "http_req_duration": [{ threshold: "max<10000", abortOnFail: abort_on_fail }], 
+        "http_req_duration": [
+            { threshold: "p(95)<2000", abortOnFail: abort_on_fail },  // 95% of requests should complete within 2s
+            { threshold: "max<5000", abortOnFail: abort_on_fail }     // no request should take more than 5s
+        ],

17-19: Consider adding multiple stages for better load testing.

The current single-stage configuration might not effectively test system behavior under varying loads. Consider adding multiple stages to test ramp-up, steady state, and ramp-down behavior.

     stages: [
-        { duration: stages_duration, target: stages_target }, // simulate ramp-up of traffic from 1 to stages_target users over stages_duration
+        { duration: '30s', target: stages_target/2 },     // ramp-up to 50% load
+        { duration: '30s', target: stages_target },       // ramp-up to full load
+        { duration: stages_duration, target: stages_target }, // maintain load
+        { duration: '30s', target: 0 },                   // ramp-down
     ],

22-24: Add error handling and logging.

Consider adding try-catch and logging to better track test execution and failures.

 export default function() {
-    createDialog(randomItem(serviceOwners), randomItem(endUsers), traceCalls); 
+    try {
+        const serviceOwner = randomItem(serviceOwners);
+        const endUser = randomItem(endUsers);
+        
+        if (traceCalls) {
+            console.log(`Creating dialog: ServiceOwner=${serviceOwner.id}, EndUser=${endUser.id}`);
+        }
+        
+        createDialog(serviceOwner, endUser, traceCalls);
+    } catch (error) {
+        console.error('Failed to create dialog:', error);
+        throw error;  // Re-throw to mark the iteration as failed
+    }
 }
.github/workflows/dispatch-k6-breakpoint.yml (2)

39-43: Remove Trailing Spaces
Line 43 has trailing spaces after the type: boolean declaration. Removing these spaces improves code cleanliness and prevents YAML lint warnings.
[minor]

-        type: boolean  
+        type: boolean
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 43-43: trailing spaces

(trailing-spaces)


73-73: Remove Extra Blank Line
Static analysis indicates that there is an extra blank line at line 73. Removing this blank line will satisfy lint rules regarding unnecessary empty lines.

🧰 Tools
🪛 YAMLlint (1.35.1)

[warning] 73-73: too many blank lines

(1 > 0) (empty-lines)

.github/workflows/workflow-run-k6-breakpoint.yml (1)

72-78: Quote Variables in the k6 Test Run Command
The run command on line 77 passes several variables to the shell script. To prevent issues with word splitting and globbing (as flagged by shellcheck), it is recommended to wrap all variable expansions in double quotes. This ensures robust handling of values that might contain spaces.

- ./tests/k6/tests/scripts/run-test-in-k8s.sh -f ${{ inputs.testSuitePath }} -c $k6configName -n $k6configName -v ${{ inputs.targetVus }} -d ${{ inputs.duration }} -p ${{ inputs.parallelism }} -a ${{ inputs.abortOnFail }} -b
+ ./tests/k6/tests/scripts/run-test-in-k8s.sh -f "${{ inputs.testSuitePath }}" -c "$k6configName" -n "$k6configName" -v "${{ inputs.targetVus }}" -d "${{ inputs.duration }}" -p "${{ inputs.parallelism }}" -a "${{ inputs.abortOnFail }}" -b
🧰 Tools
🪛 actionlint (1.7.4)

73-73: shellcheck reported issue in this script: SC2086:info:4:79: Double quote to prevent globbing and word splitting

(shellcheck)


73-73: shellcheck reported issue in this script: SC2086:info:4:96: Double quote to prevent globbing and word splitting

(shellcheck)

tests/k6/tests/serviceowner/performance/createTransmissionsBreakpoint.js (2)

5-11: Consider adding input validation for environment variables.

The environment variables are used directly without validation. Consider adding validation for numeric values and ensuring they are within reasonable ranges.

-const numberOfTransmissions = (__ENV.numberOfTransmissions ?? '10');
-const maxTransmissionsInThread = (__ENV.maxTransmissionsInThread ?? '100');
-const stages_target = (__ENV.stages_target ?? '5');
+const numberOfTransmissions = parseInt(__ENV.numberOfTransmissions ?? '10', 10);
+if (isNaN(numberOfTransmissions) || numberOfTransmissions <= 0) {
+  throw new Error('numberOfTransmissions must be a positive number');
+}
+const maxTransmissionsInThread = parseInt(__ENV.maxTransmissionsInThread ?? '100', 10);
+if (isNaN(maxTransmissionsInThread) || maxTransmissionsInThread <= 0) {
+  throw new Error('maxTransmissionsInThread must be a positive number');
+}
+const stages_target = parseInt(__ENV.stages_target ?? '5', 10);
+if (isNaN(stages_target) || stages_target <= 0) {
+  throw new Error('stages_target must be a positive number');
+}

13-25: Consider adding more granular thresholds.

The current thresholds only check for maximum duration. Consider adding p95 and error rate thresholds for better performance monitoring.

 thresholds: {
-    "http_req_duration{name:create dialog}": [{ threshold: "max<10000", abortOnFail: abort_on_fail }],
-    "http_req_duration{name:create transmission}": [{ threshold: "max<10000", abortOnFail: abort_on_fail }],
+    "http_req_duration{name:create dialog}": [
+        { threshold: "max<10000", abortOnFail: abort_on_fail },
+        { threshold: "p(95)<5000", abortOnFail: abort_on_fail }
+    ],
+    "http_req_duration{name:create transmission}": [
+        { threshold: "max<10000", abortOnFail: abort_on_fail },
+        { threshold: "p(95)<5000", abortOnFail: abort_on_fail }
+    ],
+    "http_req_failed": [{ threshold: "rate<0.01", abortOnFail: abort_on_fail }],
tests/k6/tests/enduser/performance/enduserSearchBreakpoint.js (2)

30-33: Consider adding a ramp-down stage.

The current configuration only has a ramp-up stage. Adding a ramp-down stage can help observe system behavior during load reduction.

 stages: [
     { duration: stages_duration, target: stages_target }, // simulate ramp-up of traffic from 1 to stages_target users over stages_duration
+    { duration: stages_duration, target: 0 }, // ramp-down to 0 users
 ],

12-29: Consider grouping related thresholds.

The thresholds section contains many similar endpoints. Consider grouping them using tags for better organization and maintenance.

 thresholds: {
-    "http_req_duration{name:enduser search}": [{ threshold: "max<5000", abortOnFail: abort_on_fail }],
-    "http_req_duration{name:get dialog}": [{ threshold: "max<5000", abortOnFail: abort_on_fail }],
-    "http_req_duration{name:get dialog activities}": [{ threshold: "max<5000", abortOnFail: abort_on_fail }],
+    "http_req_duration{group:search}": [{ threshold: "max<5000", abortOnFail: abort_on_fail }],
+    "http_req_duration{group:dialog}": [{ threshold: "max<5000", abortOnFail: abort_on_fail }],
+    "http_req_duration{group:activities}": [{ threshold: "max<5000", abortOnFail: abort_on_fail }],
tests/k6/tests/performancetest_common/readTestdata.js (1)

55-71: Consider adding input validation and documentation.

The endUsersPart function lacks input validation and JSDoc documentation.

+/**
+ * Splits end users array into parts based on the total number of virtual users.
+ * @param {number} totalVus - Total number of virtual users
+ * @param {number} vuId - Current virtual user ID (1-based)
+ * @returns {Array} Array of end users for the current virtual user
+ * @throws {Error} If totalVus or vuId are invalid
+ */
 export function endUsersPart(totalVus, vuId) {
+  if (!Number.isInteger(totalVus) || totalVus <= 0) {
+    throw new Error('totalVus must be a positive integer');
+  }
+  if (!Number.isInteger(vuId) || vuId <= 0 || vuId > totalVus) {
+    throw new Error('vuId must be a positive integer not greater than totalVus');
+  }
   const endUsersLength = endUsers.length;
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26abb48 and 628e954.

📒 Files selected for processing (8)
  • .github/workflows/dispatch-k6-breakpoint.yml (1 hunks)
  • .github/workflows/workflow-run-k6-breakpoint.yml (1 hunks)
  • tests/k6/tests/enduser/performance/enduserSearchBreakpoint.js (1 hunks)
  • tests/k6/tests/performancetest_common/readTestdata.js (1 hunks)
  • tests/k6/tests/scripts/run-test-in-k8s.sh (5 hunks)
  • tests/k6/tests/serviceowner/performance/createDialogBreakpoint.js (1 hunks)
  • tests/k6/tests/serviceowner/performance/createTransmissionsBreakpoint.js (1 hunks)
  • tests/k6/tests/serviceowner/performance/serviceOwnerSearchBreakpoint.js (1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/dispatch-k6-breakpoint.yml

[error] 43-43: trailing spaces

(trailing-spaces)


[warning] 73-73: too many blank lines

(1 > 0) (empty-lines)

🪛 actionlint (1.7.4)
.github/workflows/workflow-run-k6-breakpoint.yml

73-73: shellcheck reported issue in this script: SC2086:info:4:79: Double quote to prevent globbing and word splitting

(shellcheck)


73-73: shellcheck reported issue in this script: SC2086:info:4:96: Double quote to prevent globbing and word splitting

(shellcheck)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Dry run deploy infrastructure / Deploy to test
  • GitHub Check: build / build-and-test
🔇 Additional comments (13)
tests/k6/tests/scripts/run-test-in-k8s.sh (3)

118-119: Consistent Naming Through Lowercase Conversion
Converting the name and configmapname variables to lowercase helps maintain consistency across resource names. This addition is clear and well implemented.


123-126: Conditional Archive Arguments for Breakpoint Tests
The update that sets archive_args with breakpoint‑specific environment variables (stages_target, stages_duration, and abort_on_fail) when the breakpoint flag is enabled looks correct. Ensure that the values for vus and duration (which are required arguments) are appropriately validated in all scenarios since they are used both here and in the traditional configuration path.


142-145: Dynamic Test Run Arguments Based on Breakpoint Flag
The script conditionally assigns the arguments variable for the config file based on the state of the breakpoint flag. Omitting the --vus and --duration options in breakpoint mode appears to be the intended behavior. Please verify that this change is aligned with the overall design of your performance tests and that any necessary documentation is updated accordingly.

tests/k6/tests/serviceowner/performance/serviceOwnerSearchBreakpoint.js (1)

1-45: Well-structured performance test implementation!

The implementation follows k6 best practices with good configuration options and proper test setup. The suggested improvements are optional enhancements to an already solid foundation.

.github/workflows/dispatch-k6-breakpoint.yml (3)

1-38: Input Parameter Definitions Look Solid
The definition of the workflow inputs—including types, descriptions, required flags, and default values—is well organized and clear.


44-54: Test Suite Path as a Choice
The testSuitePath input is correctly configured as a choice with predefined options, which helps prevent misconfiguration. This approach supports proper validation for the test suite paths.


55-72: Job and Run-name Configuration is Correct
The dynamic run-name and the job configuration correctly reference the inputs and secrets. The use of fromJson for numeric inputs appears intended to ensure proper type conversion.

.github/workflows/workflow-run-k6-breakpoint.yml (6)

1-26: Workflow Call Inputs and Secrets are Configured Properly
The workflow call inputs and secret definitions are clear and adhere to best practices. Each required input is explicitly declared with its type, ensuring that the workflow receives the expected parameters.


27-38: Permissions Setup is Correct
The permissions block correctly requests the id-token write and contents read permissions, allowing for secure and limited access to necessary resources.


39-51: Job Environment and Checkout Steps
The job configuration, including the runner environment, the checkout of code, and the OIDC login step, is implemented properly. Ensuring that the authentication to Azure is in place is critical, and these steps meet that requirement.


52-69: Robust Kubeconfig Population Step
The “Populate kubeconfig with k6 context” step incorporates error handling for each command, ensuring that failures are caught early. This defensive programming is important in CI/CD pipelines.


70-71: k6 Setup Step is Concise
The step to set up k6 using the Grafana action is straightforward and adheres to recommended practices.


79-83: Environment Variable Propagation is Handled Well
The environment variables for the test execution (including API environment, version, and token generator secrets) are correctly mapped from the inputs and secrets.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
.github/workflows/dispatch-k6-breakpoint.yml (2)

43-43: Remove Trailing Whitespace

Static analysis flagged trailing spaces on line 43. Please remove any extra whitespace to adhere to YAML linting guidelines.

Diff suggestion:

-        default: 'v1'␣␣
+        default: 'v1'
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 43-43: trailing spaces

(trailing-spaces)


74-74: Remove Excess Blank Line

A warning indicates that there is one too many blank lines (line 74). Please remove the extra blank line to improve code style consistency.

🧰 Tools
🪛 YAMLlint (1.35.1)

[warning] 74-74: too many blank lines

(1 > 0) (empty-lines)

.github/workflows/workflow-run-k6-performance.yml (1)

80-80: Trim Trailing Whitespace in Command Invocation

There is trailing whitespace at the end of the command in line 80. Removing it will help avoid potential issues or linting warnings.

Diff suggestion:

-        ./tests/k6/tests/scripts/run-test-in-k8s.sh -f ${{ inputs.testSuitePath }} -c $k6configName -n $k6configName -v ${{ inputs.vus }} -d ${{ inputs.duration }} -p ${{ inputs.parallelism }} -a ${{ inputs.abortOnFail }} -b ${{ inputs.breakpoint }} 
+        ./tests/k6/tests/scripts/run-test-in-k8s.sh -f ${{ inputs.testSuitePath }} -c $k6configName -n $k6configName -v ${{ inputs.vus }} -d ${{ inputs.duration }} -p ${{ inputs.parallelism }} -a ${{ inputs.abortOnFail }} -b ${{ inputs.breakpoint }}
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 628e954 and d9e6582.

📒 Files selected for processing (4)
  • .github/workflows/dispatch-k6-breakpoint.yml (1 hunks)
  • .github/workflows/dispatch-k6-performance.yml (1 hunks)
  • .github/workflows/workflow-run-k6-performance.yml (2 hunks)
  • tests/k6/tests/scripts/run-test-in-k8s.sh (5 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/workflow-run-k6-performance.yml

[error] 80-80: trailing spaces

(trailing-spaces)

.github/workflows/dispatch-k6-breakpoint.yml

[error] 43-43: trailing spaces

(trailing-spaces)


[warning] 74-74: too many blank lines

(1 > 0) (empty-lines)

🔇 Additional comments (9)
.github/workflows/dispatch-k6-performance.yml (1)

69-71: New Default Parameters for Breakpoint Tests

The new keys breakpoint: false and abortOnFail: false are introduced and correctly passed as inputs to the downstream workflow. This ensures that, by default, performance tests are executed without breakpoint testing and without aborting on failure unless explicitly overridden.

.github/workflows/dispatch-k6-breakpoint.yml (2)

1-54: Establishing a New Workflow for Breakpoint Tests

The workflow for dispatching k6 breakpoint tests is well defined. All the input parameters (e.g., apiVersion, environment, tag, targetVus, duration, parallelism, abortOnFail, and testSuitePath) are clearly specified with appropriate types, defaults, and options. Notably, the targetVus input is converted via fromJson when passed to the underlying workflow, which is consistent with number inputs in other workflows.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 43-43: trailing spaces

(trailing-spaces)


55-73: Passing Breakpoint-Specific Options

Within the job configuration, the parameters are correctly forwarded:

  • abortOnFail is passed directly from the input.
  • breakpoint is hardcoded to true, which is acceptable since this workflow is dedicated to breakpoint testing.

This clear delineation matches the PR’s objective of adding breakpoint tests.

.github/workflows/workflow-run-k6-performance.yml (1)

24-29: Introducing New Boolean Inputs

The addition of the breakpoint and abortOnFail inputs (both required and of type boolean) extends the configuration options for K6 tests. Their inclusion in the workflow call inputs is clear and consistent with the new dispatch workflows.

tests/k6/tests/scripts/run-test-in-k8s.sh (5)

24-27: Enhancing Help Documentation for New Flags

The help function now includes entries for -b, --breakpoint and -a, --abort, which inform users about the new flags. This update enhances the script’s usability by clearly communicating the available options for breakpoint tests and abort behavior.


56-57: Initialization of Breakpoint Variables

Initializing breakpoint and abort_on_fail to false at the start is appropriate and sets a clear default behavior.


88-95: Flag Argument Parsing: Consistency Check

The new argument parsing for the flags -b/--breakpoint and -a/--abort requires an accompanying parameter (using $2), which is different from typical boolean flags that toggle state when present. This pattern has been flagged previously. It may be preferable to either:

  • Change these to true flag options (i.e., not require a parameter) so that their presence alone sets the variable to true, or
  • Clearly document that these flags require an explicit true/false value.

If the current behavior is intentional, please ensure that it is prominently documented in the usage instructions.


124-126: Conditional Archive Arguments Logic

The conditional setting of archive_args based on the $breakpoint flag correctly tailors the execution of k6 archive for breakpoint tests. This logic is clear and meets the functional requirements.


142-145: Adjusting Test Run Arguments for Breakpoint Tests

The script defines the arguments variable differently depending on whether breakpoint mode is active, omitting the --vus and --duration options when $breakpoint is true. This conditional logic is well implemented and aligns with the purpose of breakpoint tests.

arealmaas
arealmaas previously approved these changes Feb 4, 2025
renovate bot and others added 2 commits February 4, 2025 14:45
This PR contains the following updates:

| Resource | Change |
|---|---|
| Microsoft.App/managedEnvironments | `2024-02-02-preview` ->
`2024-03-01` |
| Microsoft.AppConfiguration/configurationStores | `2023-03-01` ->
`2024-05-01` |
| Microsoft.Cache/Redis | `2023-08-01` -> `2024-11-01` |
| Microsoft.Network/privateDnsZones | `2020-06-01` -> `2024-06-01` |
| Microsoft.Network/privateDnsZones/A | `2020-06-01` -> `2024-06-01` |
| Microsoft.Network/privateDnsZones/virtualNetworkLinks | `2020-06-01`
-> `2024-06-01` |
| Microsoft.Resources/resourceGroups | `2024-03-01` -> `2024-11-01` |

---

### Configuration

📅 **Schedule**: Branch creation - "before 7am on Sunday,before 7am on
Wednesday" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config
help](https://redirect.github.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/Altinn/dialogporten).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS43Mi4yIiwidXBkYXRlZEluVmVyIjoiMzkuMTA3LjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Ole Jørgen Skogstad <skogstad@softis.net>
Co-authored-by: Are Almaas <arealmaas@gmail.com>
@dagfinno dagfinno merged commit fe93b20 into main Feb 4, 2025
23 checks passed
@dagfinno dagfinno deleted the performance/breakpoint branch February 4, 2025 13:53
oskogstad pushed a commit that referenced this pull request Feb 5, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.48.3](v1.48.2...v1.48.3)
(2025-02-05)


### Miscellaneous Chores

* **deps:** bump vitest from 3.0.4 to 3.0.5
([#1798](#1798))
([7c306fd](7c306fd))
* **deps:** update bicep dependencies (major)
([#1621](#1621))
([6fef560](6fef560))
* **deps:** update dotnet monorepo
([#1800](#1800))
([0d08537](0d08537))
* **deps:** update masstransit monorepo to 8.3.5
([#1801](#1801))
([3f35e0f](3f35e0f))
* **graphql:** Remove custom OTEL event listener
([#1797](#1797))
([56adb3f](56adb3f))
* **performance:** Adding breakpoint tests
([#1793](#1793))
([fe93b20](fe93b20))
* Remove CDC and obsolete version property from docker compose
([#1796](#1796))
([663734c](663734c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants