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: add minimum test duration for report to display graphs #1501

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

heitortsergent
Copy link
Collaborator

What?

Add the minimum test duration required for graphs to be displayed in the report generated by the web dashboard feature.

Checklist

Please fill in this template:

  • I have used a meaningful title for the PR.
  • I have described the changes I've made in the "What?" section above.
  • I have performed a self-review of my changes.
  • I have run the make docs command locally and verified that the changes look good.

If updating the documentation for the most recent release of k6:

  • I have made my changes in the docs/sources/next folder of the documentation.
  • I have reflected my changes in the docs/sources/v{most_recent_release} folder of the documentation.
  • I have reflected my changes in the relevant (e.g. when correcting a documentation error) folders of the previous k6 versions of the documentation.

If updating the documentation for the next release of k6:

  • I have made my changes in the docs/sources/next folder of the documentation.

Related PR(s)/Issue(s)

Closes #1499.

@heitortsergent
Copy link
Collaborator Author

@szkiba this is from our Slack thread, please let me know if this is accurate. 🙏

@szkiba
Copy link
Contributor

szkiba commented Feb 3, 2024

@heitortsergent it would be a bit more precise if write something like this instead of 30 sec: "3 times the aggregation period (default 10sec). "
This is how 30 sec come into the picture

Copy link
Contributor

@szkiba szkiba left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -65,6 +65,8 @@ To automatically generate a report from the command line once the test finishes
K6_WEB_DASHBOARD=true K6_WEB_DASHBOARD_EXPORT=html-report.html k6 run script.js
```

The report only includes graphs if the test duration is over thirty seconds.
Copy link
Contributor

Choose a reason for hiding this comment

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

@heitortsergent it would be a bit more precise if write something like this instead of 30 sec: "3 times the aggregation period (default 10sec). "
This is how 30 sec come into the picture

@@ -65,6 +65,12 @@ To automatically generate a report from the command line once the test finishes
K6_WEB_DASHBOARD=true K6_WEB_DASHBOARD_EXPORT=html-report.html k6 run script.js
```

{{< admonition type="note" >}}

The report only includes graphs if the test duration is greater than three times the aggregation period value, set by the `K6_WEB_DASHBOARD_PERIOD` variable.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@szkiba I updated the explanation based on your comment, and also switched it to an admonition so it's easier for readers to see this.

What do you think? 🤓

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect :)
maybe I would mention 30sec as the default value in parentheses, just so the user doesn't have to search)

Copy link
Contributor

@szkiba szkiba left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -65,6 +65,12 @@ To automatically generate a report from the command line once the test finishes
K6_WEB_DASHBOARD=true K6_WEB_DASHBOARD_EXPORT=html-report.html k6 run script.js
```

{{< admonition type="note" >}}

The report only includes graphs if the test duration is greater than three times the aggregation period value, set by the `K6_WEB_DASHBOARD_PERIOD` variable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect :)
maybe I would mention 30sec as the default value in parentheses, just so the user doesn't have to search)

@heitortsergent heitortsergent merged commit e78d2ec into main Feb 7, 2024
6 checks passed
@heitortsergent heitortsergent deleted the chore/update-dashboard branch February 7, 2024 20:28
@heitortsergent
Copy link
Collaborator Author

@szkiba I wrote another version mentioning the 30s because of the default value, and I was just having a hard time making it clear and concise... So I decided to just leave it as is for now, that way if we decide to change the default iteration value, we don't have to remember to update this admonition too. :D

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.

k6-dashboard: add note that test needs to be >30s for graph to be displayed in export
2 participants