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

Add more details to HTML report #256

Merged
merged 17 commits into from
Sep 16, 2020
Merged

Conversation

lptr
Copy link
Member

@lptr lptr commented Sep 15, 2020

Add some extra information like the system properties and Java home.

Improves #161.

Reports:

test-html-report.zip

image

@lptr lptr added this to the 0.15.0 milestone Sep 15, 2020
@lptr lptr self-assigned this Sep 15, 2020
@lptr lptr force-pushed the lptr/add-details-to-html-report branch 3 times, most recently from eec05b4 to 957ad7f Compare September 15, 2020 14:37
@@ -65,7 +65,7 @@ public static String safeFileName(String name) {

@Override
public String getBuildToolDisplayName() {
return buildConfiguration.getGradleVersion().getVersion();
return buildConfiguration.getGradleVersion().toString();
Copy link
Member Author

Choose a reason for hiding this comment

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

So the build tool for Gradle scenarios gets displayed as Gradle 6.7, not 6.7.

@lptr lptr force-pushed the lptr/add-details-to-html-report branch from 1142b34 to 3ba8a23 Compare September 15, 2020 15:04
@lptr lptr marked this pull request as ready for review September 15, 2020 15:04
@lptr lptr requested a review from wolfs September 15, 2020 15:04
@lptr lptr force-pushed the lptr/add-details-to-html-report branch from 3ba8a23 to d944a01 Compare September 15, 2020 15:30
Copy link
Member

@wolfs wolfs left a comment

Choose a reason for hiding this comment

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

Nice! I added some comments.

json.addProperty("name", scenario.getName());
json.addProperty("title", scenario.getTitle());
json.addProperty("displayName", scenario.getDisplayName());
json.addProperty("buildTool", scenario.getBuildToolDisplayName());
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the build tool should only be Gradle and not include the version number given that we have the version number in a separate property?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, but let's do it in a separate PR.

@lptr lptr requested a review from wolfs September 16, 2020 14:00
@lptr
Copy link
Member Author

lptr commented Sep 16, 2020

We are now showing the build tool in parenthesis after the scenario title if and only if there are different build tools invoked during the benchmark:

image

If all scenarios are executed with the same build tool (in the case of Gradle it means the same version), then the build tool version is only shown in the details view after pressing +.

image

@lptr lptr requested a review from wolfs September 16, 2020 15:47
Copy link
Member

@wolfs wolfs left a comment

Choose a reason for hiding this comment

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

LGTM!

@lptr lptr merged commit 65f4fbe into master Sep 16, 2020
@lptr lptr deleted the lptr/add-details-to-html-report branch September 16, 2020 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants