Skip to content

Add generate diff command to create html difference reports#199

Merged
AleksandarSavchev merged 8 commits intogardener:mainfrom
AleksandarSavchev:generate-html-diff
Apr 1, 2024
Merged

Add generate diff command to create html difference reports#199
AleksandarSavchev merged 8 commits intogardener:mainfrom
AleksandarSavchev:generate-html-diff

Conversation

@AleksandarSavchev
Copy link
Copy Markdown
Member

@AleksandarSavchev AleksandarSavchev commented Mar 26, 2024

What this PR does / why we need it:
This PR adds a new diki report generate diff command that creates a html difference report with 1 or more json difference reports. The --identity-attributes is required, it's value is a list of key=value pairs, where the keys are the IDs for the providers that will be present in the generated difference report and the values are metadata attributes to be used as IDs.

Which issue(s) this PR fixes:
Fixes #159

Special notes for your reviewer:

Release note:

A new command `diki report generate diff` that converts one or more `json` difference reports into a single `html` difference report was introduced.

@gardener-robot gardener-robot added needs/review size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 26, 2024
@AleksandarSavchev AleksandarSavchev marked this pull request as ready for review March 29, 2024 13:11
@AleksandarSavchev AleksandarSavchev requested a review from a team as a code owner March 29, 2024 13:11
@gardener-robot
Copy link
Copy Markdown
Contributor

@dimityrmirchev You have pull request review open invite, please check

Copy link
Copy Markdown
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

Looks, good. Only some cosmetic suggestions.

Comment thread README.md Outdated
#### Difference

Diki can generate a json containing the difference between 2 output files of `diki run` executions. This can help to identify improvements (or regressions).
Diki can generate a json containing the difference between 2 output files of `diki run` executions. This can help to identify improvements (or regressions). A human readable html difference report can also be generated from the json difference reports.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Diki can generate a json containing the difference between 2 output files of `diki run` executions. This can help to identify improvements (or regressions). A human readable html difference report can also be generated from the json difference reports.
Diki can generate a json containing the difference between 2 output files of `diki run` executions. This can help to identify improvements (or regressions). A human readable html difference report can be generated from the difference reports.

Comment thread README.md Outdated
diki report diff --title=Title --old=output1.json --new=output2.json --output=difference.json
```

- Generate html difference report from 1 or more json difference reports.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- Generate html difference report from 1 or more json difference reports.
- Combine one or more json difference reports to an html report.

Comment thread README.md Outdated

- Generate html difference report from 1 or more json difference reports.
```bash
diki report generate diff --unique-attributes=gardener=id --output=difference.html difference1.json difference2.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
diki report generate diff --unique-attributes=gardener=id --output=difference.html difference1.json difference2.json
diki report generate diff --identity-attributes=gardener=id --output=difference.html difference1.json difference2.json

Comment thread cmd/diki/app/app.go Outdated
Comment on lines +117 to +118
Short: "Generate diff converts difference reports to html.",
Long: "Generate diff converts difference reports to html.",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Short: "Generate diff converts difference reports to html.",
Long: "Generate diff converts difference reports to html.",
Short: "Generate diff combines difference reports into an html report.",
Long: "Generate diff converts difference reports to html.",

Comment thread cmd/diki/app/app.go Outdated
func addReportDiffFlags(cmd *cobra.Command, opts *diffOptions) {
cmd.PersistentFlags().StringVar(&opts.oldReport, "old", "", "Old report path.")
cmd.PersistentFlags().StringVar(&opts.newReport, "new", "", "New report path.")
cmd.PersistentFlags().StringVar(&opts.title, "title", "", "Title for difference report.")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
cmd.PersistentFlags().StringVar(&opts.title, "title", "", "Title for difference report.")
cmd.PersistentFlags().StringVar(&opts.title, "title", "", "The title of a difference report.")

Comment thread cmd/diki/app/app.go Outdated
}

func addReportGenerateDiffFlags(cmd *cobra.Command, opts *generateDiffOptions) {
cmd.PersistentFlags().Var(cliflag.NewMapStringString(&opts.uniqueAttributes), "unique-attributes", "The keys are the IDs for the providers that will be present in the generated difference report and the values are metadata attributes to be used as IDs")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
cmd.PersistentFlags().Var(cliflag.NewMapStringString(&opts.uniqueAttributes), "unique-attributes", "The keys are the IDs for the providers that will be present in the generated difference report and the values are metadata attributes to be used as IDs")
cmd.PersistentFlags().Var(cliflag.NewMapStringString(&opts.uniqueAttributes), "identity-attributes", "The keys are the IDs of the providers that will be present in the generated difference report and the values are metadata attributes to be used as identifiers.")

Comment thread cmd/diki/app/app.go Outdated
Comment on lines +181 to +184
htlmRenderer, err := report.NewHTMLRenderer()
if err != nil {
return fmt.Errorf("failed to initialize renderer: %w", err)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be moved closer to its usage.

Comment thread cmd/diki/app/app.go Outdated
}

type generateDiffOptions struct {
uniqueAttributes map[string]string
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be renamed

Comment thread pkg/report/difference.go Outdated
// Difference contains the difference between 2 reports.
type Difference struct {
// DifferenceReportWrapper wraps DifferenceReports and additional attributes needed for html rendering.
type DifferenceReportWrapper struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
type DifferenceReportWrapper struct {
type DifferenceReportsWrapper struct {

Comment thread pkg/report/difference.go Outdated
return template.HTML(summaryBuilder.String()) //nolint:gosec
}

func getDiffUniqAttrsText(providerDiff ProviderDifference, key string) string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
func getDiffUniqAttrsText(providerDiff ProviderDifference, key string) string {
func getProviderDiffIDText(providerDiff ProviderDifference, key string) string {

Copy link
Copy Markdown
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Diff between 2 diki reports

5 participants