Skip to content

Conversation

@bestmike007
Copy link
Contributor

What changed? Why?

  • Add a rpc_replayer tool for recording rpc responses
  • Run functional tests in GitHub Actions

How did you test the change?

GitHub Actions

@bestmike007 bestmike007 requested a review from a team as a code owner November 7, 2023 19:10
Signed-off-by: bestmike007 <i@bestmike007.com>
Copy link
Contributor

@jiezhang jiezhang left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! We heavily rely on these functional tests at Coinbase to catch regressions, but we didn't have a good way to run them in this open source repo. Nice work!

fmt.Printf("%+v, cache path: %v\n", jsonBody, cachePath)
cache, err := os.ReadFile(cachePath)
if err == nil {
_, _ = w.Write(cache)
Copy link
Contributor

Choose a reason for hiding this comment

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

do not ignore error

http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
_ = os.WriteFile(cachePath, result, 0o644)
Copy link
Contributor

Choose a reason for hiding this comment

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

do not ignore error

@@ -0,0 +1,137 @@
package main
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update README and document how to re-generate the fixtures?

return
}
_, _ = w.Write(resultBody)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add an empty line at the end?

image

return
}
cachePath := path.Join(
baseFolder, "seed", chain,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
baseFolder, "seed", chain,
baseFolder, "fixtures", chain,

@bestmike007 bestmike007 marked this pull request as draft November 9, 2023 18:29
@leozc
Copy link
Contributor

leozc commented Jan 6, 2024

@bestmike007 FYI it seems there is some conflicts

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants