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

Refactor Hyperfoil document #234

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

diegolovison
Copy link
Contributor

No description provided.

@diegolovison diegolovison changed the title Refactor hyperfoil document Refactor Hyperfoil document Jan 13, 2025
Copy link
Collaborator

@willr3 willr3 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 this revision a bit harder to understand. I will take a look at the new rendered version; perhaps looking at just the diff is why I felt so confused.


Run with and replace `targethost` value with your needs
```
targethost=$USER@localhost
Copy link
Collaborator

Choose a reason for hiding this comment

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

why does the guide introduce the extra step of creating the environment variable? Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

The previous tutorial created a qDup yaml that downloaded Quarkus getting started and ran a single curl against the `quarkus dev` process. We are going to start where it left off:

```yaml
== Running the experiment
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a sentence after the heading as a code section immediately after the heading can leave a user to scroll past the code section to find out why the code section exists then scrolling back up to the code section to interact with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

```

== Splitting the roles
The previous tutorial created an `example` role to run all of ours scripts. This works but a proper performance test will normally isolate the load generation (currently `curl`) from the process we are testing (currently `quarkus dev`).
qDup isolates scripts by assigning them to roles that are running on different `hosts`. To isolate `test-endoing` from `getting-started` we need to introduce a second role. Let us call it `hyperfoil`. The `roles` section will now look like the following:
The previous tutorial created a role to run all of ours scripts. This works but a proper performance test will normally isolate the load generator (`hyperfoil`) from the process we are testing (`quarkus dev`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

(e.g. hyperfoil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed (hyperfoil) and (quarkus dev)


The first command is to `wget` the hyperfoil release but we are going to start with `cd /tmp` like we did in `getting-started` so that we don't add files to the current home directory.
==== setup-hyperfoil
It will download, install Hyperfoil locally and configure the `single-request.hf.yaml` file to use `http://localhost:8080` instead of `http://hyperfoil.io`
Copy link
Collaborator

Choose a reason for hiding this comment

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

The first sentence after a new heading should probabably use a proper noun as the topic of the first sentence instead of a pronoun. Starting with it will force a reader to go back to the previous section to identify the topic of the new section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

```

The next command is to run `> bin/cli.sh`. Test that in terminal and notice how it starts an interactive shell and changes the prompt.
==== test-endpoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the benefit of breaking this section into multiple sub-sections with the ==== decorator? I do not understand how breaking each script into a different heading section helps the readability. Was it difficult to read in the previous format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the benefit of breaking this section into multiple sub-sections with the ==== decorator?

It is clear that ==== test-endpoint belongs to === Hyperfoil role

Was it difficult to read in the previous format?

It wasn't.

Here is an example:

image

@diegolovison
Copy link
Contributor Author

I will take a look at the new rendered version

For this situation, the rendered version is easy to read.


I have addressed all the comments. If you agree with the proposed solution, don't forget to squash it before merging.

image

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