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.

Sign up for GitHub

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

Jump to bottom

Remove passing of anonymous visualize function #19089

Closed
serathius opened this issue Dec 19, 2024 · 7 comments · Fixed by #19370 · May be fixed by #19094
Closed

Remove passing of anonymous visualize function #19089

serathius opened this issue Dec 19, 2024 · 7 comments · Fixed by #19370 · May be fixed by #19094
Assignees
@VidhuSarwal
Labels
area/robustness-testing good first issue help wanted type/cleanup

Comments

@serathius
Copy link
Member

What would you like to be added?

Before linearizationInfo was a private struct we were not able to return the results up the call stack. So I wrapped the function I needed and passed it like

etcd/tests/robustness/validate/operations.go

Lines 49 to 56 in e4d1da7

return result, func(path string) error {
lg.Info("Saving visualization", zap.String("path", path))
err := porcupine.VisualizePath(model.NonDeterministicModel, info, path)
if err != nil {
return fmt.Errorf("failed to visualize, err: %w", err)
}
return nil
}

With https://github.com/etcd-io/etcd/blob/main/tests/robustness/validate/operations.go#L49-L56 making the struct piblic we can simply replace it with passing it linearizationInfo.

Why is this needed?

Cleanup code
@serathius
Copy link
Member Author

As VisualizePath function needs also model as argument, maybe we can wrap it in struct with Visualize method:

type Results struct {
  info pourcupine.LinearizationInfo
  model pourcupine.Model
}

@lhajouji
Copy link

Hello @serathius , i'd like to work on this ?
Could you assign it to me, Thanks in advance.

@VidhuSarwal VidhuSarwal mentioned this issue Dec 20, 2024
Open
@VidhuSarwal
Copy link

Hi, I submitted a PR please review. Thanks.

@siyuanfoundation
Copy link
Contributor

/assign @VidhuSarwal

@zzzk1
Copy link

zzzk1 commented Jan 28, 2025

@VidhuSarwal Are you still working on this? If you have no time, i would like to help.😊

@VidhuSarwal
Copy link

VidhuSarwal commented Jan 29, 2025 via email

@shashwat010 shashwat010 mentioned this issue Feb 10, 2025
Merged
@shashwat010
Copy link
Contributor

Made required changes to #19094 in PR: #19327

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

@VidhuSarwal VidhuSarwal

Labels
area/robustness-testing good first issue help wanted type/cleanup
Milestone
No milestone
6 participants
@serathius @lhajouji @siyuanfoundation @zzzk1 @VidhuSarwal @shashwat010