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

Utils for tagging stages #6

Merged
merged 1 commit into from
Apr 29, 2022
Merged

Utils for tagging stages #6

merged 1 commit into from
Apr 29, 2022

Conversation

codebien
Copy link
Contributor

@codebien codebien commented Apr 20, 2022

This PR implements the code for tagging stages, adding some utils function for getting and tagging with the index (or the computed profile) of the current running stage.

It requires an xk6 build of grafana/k6#2493 for running the tests.

$ xk6 build master
$ ./k6 run tests/stages.js

@codebien codebien self-assigned this Apr 20, 2022
src/stages.js Outdated
Comment on lines 112 to 113
// TODO: should we use another tag key?
exec.vu.tags['stage'] = getStageProfile()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opinions?

Choose a reason for hiding this comment

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

My opinion is that at least tagWithCurrentStageIndex and tagWithCurrentStageProfile should not use the same key. I mean, what if a customer wants to have them both? 🤔

tests/stages.js Outdated Show resolved Hide resolved
src/stages.js Show resolved Hide resolved
src/stages.js Outdated Show resolved Hide resolved
src/stages.js Outdated
Comment on lines 112 to 113
// TODO: should we use another tag key?
exec.vu.tags['stage'] = getStageProfile()

Choose a reason for hiding this comment

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

My opinion is that at least tagWithCurrentStageIndex and tagWithCurrentStageProfile should not use the same key. I mean, what if a customer wants to have them both? 🤔

tests/stages.js Show resolved Hide resolved
src/stages.js Show resolved Hide resolved
@codebien
Copy link
Contributor Author

Added some more edge cases, and applied more refactor for making the code a bit more readable. Let me know if you have better ideas for having a better code for parseDuration. It would be good to keep at least O(N) considering the amount of time it could be invoked during a test.

Copy link
Member

@oleiade oleiade left a comment

Choose a reason for hiding this comment

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

Nickel Chrome! Great work! 🎉

@codebien codebien requested a review from javaducky April 26, 2022 13:21
@codebien
Copy link
Contributor Author

Ok, based on the approvals we are ready to open a PR on the docs and the relative PR on jslib.k6.io. This PR is dependent on the upcoming k6 v0.38.0 so I will wait for the new release before merging the PRs.

@sniku
Copy link
Collaborator

sniku commented Apr 27, 2022

Cool to see "unit tests" using k6chaijs! 🎉

@codebien codebien merged commit 5926e26 into master Apr 29, 2022
@codebien codebien deleted the stage-tagging branch April 29, 2022 09:38
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.

6 participants