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

Feat/normalize with add sleep (CLI --add-sleep) #73

Merged
merged 13 commits into from
Mar 17, 2021

Conversation

w1kman
Copy link
Contributor

@w1kman w1kman commented Mar 13, 2021

Normalize input HAR (with or without added sleep())

Background

As the HAR spec does not enforce the order of entries to be meaningful, order need to be sorted for the resulting script to resemblance the events that took place in order to produce the HAR.
Another missing piece is the time between requests. Currently sleep() need to be added manually after the conversion to script.

Sort

Sorting is done automatically as long as every entry has a startedDateTime that is not NaN when parsing with Date
If at least one entry is missing startedDateTime, the initial input archive will be returned.

Add sleep

When --add-sleep is passed to the CLI (or { addSleep: true } is passed as second parameter to liHARToK6Script/normalizeHAR), LI-HAR sleep objects will be added to entries if;

  • entrydoesn't already have a LI-HAR sleep object set
  • offset is more or equal to 500ms

CLI

--add-sleep option has been added to CLI. Default: false

liHARToK6Script

liHARToK6Script can be called with options as the second parameter.
To add sleep between request: liHARToK6Script(har, { addSleep: true })

(new) normalizeHAR

If you only want to "normalize" the HAR (without creating a k6 script), you can run normalizeHAR(har, { addSleep: true })

Copy link
Contributor Author

@w1kman w1kman left a comment

Choose a reason for hiding this comment

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

nanoid not used - Remove

package.json Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
Copy link
Contributor

@allansson allansson left a comment

Choose a reason for hiding this comment

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

I found a few things that confused me. Let me know if I've misunderstood something.

test/unit/normalize/index.test.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@legander legander left a comment

Choose a reason for hiding this comment

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

Added some comments. The one related to isValidArchive needs to be fixed.
Feel free to disregard my other comments, but you are welcome to respond (if you want).

src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
src/normalize/index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@legander legander left a comment

Choose a reason for hiding this comment

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

Had some small nitpick comments, just picking your brain a little bit. Nothing that I require being changed tho. I will run some manual testing to see that it works as expected then you will get a 👍 from me at least 😅

@legander
Copy link
Collaborator

Have run some testing now, both from CLI and from the Test builder in the cloud app. It works nicely! I think we can round the sleeps to 1 precision point, then its OK from me 🙄

@w1kman w1kman marked this pull request as draft March 16, 2021 13:15
- no "by reference"
- sleep rounded to one decimal (instead of two)
- Remove "draft"
- no "delete"
- early check of entry.startedDateTime
@w1kman w1kman marked this pull request as ready for review March 16, 2021 14:44
@w1kman w1kman requested a review from legander March 16, 2021 14:46
Copy link
Contributor

@allansson allansson left a comment

Choose a reason for hiding this comment

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

LGTM 😅

Copy link
Collaborator

@legander legander left a comment

Choose a reason for hiding this comment

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

Tested the CLI and cloud app again, it still works nicely 💯

@w1kman w1kman merged commit 39da671 into master Mar 17, 2021
@w1kman w1kman deleted the feat/normalize-with-add-sleep branch March 17, 2021 13:51
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.

None yet

3 participants