[Testing] Refactor test helpers by bryanchriswhite · Pull Request #753 · pokt-network/pocket

bryanchriswhite · 2023-05-15T05:32:20Z

Description

Issue

...

Type of change

Please mark the relevant option(s):

List of changes

...

Testing

make develop_test; if any code changes were made
Docker Compose LocalNet; if any major functionality was changed or introduced
k8s LocalNet; if any infrastructure or configuration changes were made

Required Checklist

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added, or updated, godoc format comments on touched members (see: tip.golang.org/doc/comment)
I have tested my changes using the available tooling
I have updated the corresponding CHANGELOG

If Applicable Checklist

I have updated the corresponding README(s); local and/or global
I have added tests that prove my fix is effective or that my feature works
I have added, or updated, mermaid.js diagrams in the corresponding README(s)
I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

…782) ## Description Add replace directives for `gocuke` and `go-mockdns` forks/releases until upstream PRs are merged. Includes `TECHDEBT` comments for removal. ## Issue Extracted from #753 ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [ ] Bug fix - [ ] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [x] Other: swapping test dependencies for our own forks/releases thereof ## List of changes  - Replaced `github.com/foxcpp/go-mockdns` with `github.com/pokt-network/go-mockdns v0.0.1` - Replaced `github.com/regen-network/gocuke` with `github.com/pokt-network/gocuke v0.0.1` ## Testing - [ ] `make develop_test`; if any code changes were made - [ ] `make test_e2e` on [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any code changes were made - [ ] `e2e-devnet-test` passes tests on [DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd); if any code was changed - [ ] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [ ] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made ## Required Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [ ] I have tested my changes using the available tooling - [ ] I have updated the corresponding CHANGELOG ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)

h5law · 2023-05-26T12:57:30Z

internal/testutil/keys.go

+	for i, done := 0, false; i < keyCount && !done; {
+		done = !scanner.Scan()
+		line := scanner.Text()
+		matches := privKeyManifestKeyRegex.FindStringSubmatch(line)


I wonder how this compares to this function performance wise. Decode YAML into struct and extract the keys vs. Regex

IIRC, when I wrote this, private-keys.yaml was still embedding the genesis as json so I did not pursue yaml parsing. A quick benchmark should reveal a definitive answer but I'm betting on yaml parsing being faster (perhaps with a larger memory footprint).

h5law · 2023-05-26T12:58:05Z

internal/testutil/keys_test.go

+)
+
+func TestLoadLocalnetPrivateKeys(t *testing.T) {
+	keyCount := 1000


This may need to be 3996 unless you are only checking validators

Good looking out 👍

This test only exercises extracting the keys from the file, it has no notion of node types and doesn't interact with genesis.

h5law · 2023-05-26T13:00:59Z

consensus/e2e_tests/utils_test.go

 	"context"
 	"fmt"
+	"github.com/pokt-network/pocket/internal/testutil/p2p"
+	"github.com/pokt-network/pocket/internal/testutil/persistence"


Suggested change

"github.com/pokt-network/pocket/internal/testutil/persistence"

persistence_testutil "github.com/pokt-network/pocket/internal/testutil/persistence"

@h5law you add these import aliases manually or did an IDE do it? Just curious. I explicitly (unconventionally) named the package so that an import alias wouldn't be required but I didn't have a strong opinion on whether to use one anyways.

I did I saw the errors as you were using persistence_testutil in the code and assumed this would be the import name

https://github.com/pokt-network/pocket/pull/753/files#diff-351d333685b4f6a9bb38d617a69a200daa5261f20ea31a62a7f1b4123dba9520R107

h5law · 2023-05-26T13:01:12Z

consensus/e2e_tests/utils_test.go

 import (
 	"context"
 	"fmt"
+	"github.com/pokt-network/pocket/internal/testutil/p2p"


Suggested change

"github.com/pokt-network/pocket/internal/testutil/p2p"

p2p_testutil "github.com/pokt-network/pocket/internal/testutil/p2p"

h5law · 2023-05-26T13:03:54Z

internal/testutil/bus.go

+	"github.com/pokt-network/pocket/p2p/providers/peerstore_provider"
+	"github.com/pokt-network/pocket/runtime"
+	"github.com/pokt-network/pocket/shared/messaging"
+	"github.com/regen-network/gocuke"


Suggested change

"github.com/regen-network/gocuke"

"github.com/pokt-network/gocuke"

Now you have published releases should these update?

Good question! Short answer, no.

We would prefer to use the upstream modules but need to be able to use our fork until those changes are merged upstream. To facilitate this we can use the replace directive (above) so that the import statements in the code don't need to be updated when we switch.

h5law · 2023-05-26T13:04:55Z

go.mod


 go 1.18

+replace github.com/regen-network/gocuke v0.6.2 => ../gocuke


Mentioned this below, your new pokt-network/gocuke package and same for mockdns can be used here instead?

Yup.. this branch is outdated. See https://github.com/pokt-network/pocket/pull/782/files

h5law

Did not do a thorough review of all of the new mock functions themselves, but left some comments around the keys and certain imports. Looks good so far <3

bryanchriswhite · 2023-05-26T14:02:59Z

@h5law this is still very WIP. The most up to date thinking is reflected in the notion document we'll be talking about in protocol hour today: https://pocketnetwork.notion.site/Testutils-9cba9010e18447248e9daa8a3b87e3f2

(It will also be rebased at some point)

bryanchriswhite · 2023-05-29T07:59:56Z

@h5law ⚠️ heads up, I will be force-pushing over this branch (likely more than once) so long as it's still in "draft". 🚨

FYI: The timeline item(s) that result will contain a link/ref to the old HEAD and AFAIK, until this branch is deleted, those commits should still be accessible if need be (i.e. can git checkout origin <commit>).

…782) ## Description Add replace directives for `gocuke` and `go-mockdns` forks/releases until upstream PRs are merged. Includes `TECHDEBT` comments for removal. ## Issue Extracted from #753 ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [ ] Bug fix - [ ] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [x] Other: swapping test dependencies for our own forks/releases thereof ## List of changes  - Replaced `github.com/foxcpp/go-mockdns` with `github.com/pokt-network/go-mockdns v0.0.1` - Replaced `github.com/regen-network/gocuke` with `github.com/pokt-network/gocuke v0.0.1` ## Testing - [ ] `make develop_test`; if any code changes were made - [ ] `make test_e2e` on [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any code changes were made - [ ] `e2e-devnet-test` passes tests on [DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd); if any code was changed - [ ] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [ ] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made ## Required Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [ ] I have tested my changes using the available tooling - [ ] I have updated the corresponding CHANGELOG ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)

bryanchriswhite added the testing Defining, adding, automating or modifying tests label May 15, 2023

bryanchriswhite added this to the M*: Pocket QoL (Quality of Life) milestone May 15, 2023

bryanchriswhite self-assigned this May 15, 2023

reviewpad bot added the large label May 15, 2023

Olshansk removed the large label May 17, 2023

reviewpad bot added the large Pull request is large label May 17, 2023

bryanchriswhite mentioned this pull request May 23, 2023

[Testing] [Tooling] chore: replace gocuke & go-mockdns in go.mod #782

Merged

20 tasks

h5law reviewed May 26, 2023

View reviewed changes

bryanchriswhite force-pushed the test/refactor-testutils branch from d089187 to 4dc8486 Compare May 29, 2023 12:58

bryanchriswhite force-pushed the feat/integrate-bg-router branch from ab81389 to b01996d Compare May 29, 2023 12:58

bryanchriswhite force-pushed the test/refactor-testutils branch from 4dc8486 to cb4bf21 Compare June 1, 2023 11:31

bryanchriswhite merged commit cb4bf21 into feat/integrate-bg-router Jun 1, 2023

bryanchriswhite force-pushed the feat/integrate-bg-router branch from b01996d to a8206ec Compare June 1, 2023 11:32

reviewpad bot added small Pull request is small docs and removed large Pull request is large labels Jun 1, 2023

bryanchriswhite mentioned this pull request Jun 1, 2023

[Testing] Refactor test helpers #797

Draft

20 tasks

	"github.com/pokt-network/pocket/internal/testutil/persistence"
	persistence_testutil "github.com/pokt-network/pocket/internal/testutil/persistence"

	"github.com/pokt-network/pocket/internal/testutil/p2p"
	p2p_testutil "github.com/pokt-network/pocket/internal/testutil/p2p"

	"github.com/regen-network/gocuke"
	"github.com/pokt-network/gocuke"


		go 1.18

		replace github.com/regen-network/gocuke v0.6.2 => ../gocuke

Conversation

bryanchriswhite commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue

Type of change

List of changes

Testing

Required Checklist

If Applicable Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bryanchriswhite May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bryanchriswhite May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h5law left a comment

Choose a reason for hiding this comment

Uh oh!

bryanchriswhite commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bryanchriswhite commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bryanchriswhite commented May 15, 2023 •

edited

Loading

bryanchriswhite May 26, 2023 •

edited

Loading

bryanchriswhite May 26, 2023 •

edited

Loading

bryanchriswhite commented May 26, 2023 •

edited

Loading

bryanchriswhite commented May 29, 2023 •

edited

Loading