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

feat: add basic CLI tests using Go Test #9489

Merged
merged 1 commit into from
Dec 12, 2022
Merged

Conversation

guseggert
Copy link
Contributor

This is intended as a replacement for sharness. These are vanilla Go tests which can be run in your IDE for quick iteration on end-to-end CLI tests.

This also removes IPTB by duplicating its functionality in the test harness. This isn't a big deal...IPTB's complexity is mostly around the fact that its state needs to be saved to disk in between iptb command invocations, and that it uses Go plugins to inject functionality, neither of which are relevant here.

If we merge this, we'll have to live with bifurcated tests for a while until they are all migrated. I'd recommend we self-enforce a rule that, if we need to touch a sharness test, we migrate it and one more test over to Go tests first. Then eventually we will have migrated everything.

@guseggert guseggert requested review from ajnavarro and lidel and removed request for ajnavarro December 9, 2022 13:49
test/cli/harness/harness.go Outdated Show resolved Hide resolved
test/cli/harness/harness.go Outdated Show resolved Hide resolved
test/cli/harness/harness.go Outdated Show resolved Hide resolved
test/cli/harness/cluster_node.go Outdated Show resolved Hide resolved
@guseggert guseggert force-pushed the feat/test-harness-cli branch from 89ac9de to 81cd536 Compare December 9, 2022 14:54
Comment on lines +49 to +55
node.UpdateConfig(func(cfg *config.Config) {
cfg.Routing.Type = config.NewOptionalString("custom")
cfg.Routing.Methods = config.Methods{
"find-peers": {RouterName: "TestDelegatedRouter"},
"find-providers": {RouterName: "TestDelegatedRouter"},
"get-ipns": {RouterName: "TestDelegatedRouter"},
"provide": {RouterName: "TestDelegatedRouter"},
}
})
Copy link
Contributor Author

@guseggert guseggert Dec 9, 2022

Choose a reason for hiding this comment

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

Just a side note: this test demonstrates two different ways to get at the config. This way is reading the config file straight from disk, parsing it into a config.Config struct, and then writing the result back to disk. The way below calls the CLI and deals with JSON strings. The config.Config struct is more useful for setting up config for a test, but if you want to test the ability set and read config itself, then use the CLI.

@guseggert guseggert force-pushed the feat/test-harness-cli branch 3 times, most recently from d9c9cd8 to b7797f0 Compare December 9, 2022 15:35
@guseggert guseggert force-pushed the feat/test-harness-cli branch 6 times, most recently from fc2e9c3 to 27b49a5 Compare December 12, 2022 02:33
Copy link
Member

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

Two minor changes, after that LGTM

test/cli/basic_commands_test.go Outdated Show resolved Hide resolved
test/cli/basic_commands_test.go Outdated Show resolved Hide resolved
@guseggert guseggert force-pushed the feat/test-harness-cli branch 2 times, most recently from 9f3d765 to bf65c09 Compare December 12, 2022 14:09
@guseggert guseggert requested review from ajnavarro and removed request for lidel December 12, 2022 14:09
@guseggert guseggert force-pushed the feat/test-harness-cli branch from bf65c09 to 5f89150 Compare December 12, 2022 14:18
test/cli/init_test.go Outdated Show resolved Hide resolved
@guseggert guseggert force-pushed the feat/test-harness-cli branch from 5f89150 to 3735602 Compare December 12, 2022 14:33
This is intended as a replacement for sharness. These are vanilla Go
tests which can be run in your IDE for quick iteration on end-to-end
CLI tests.

This also removes IPTB by duplicating its functionality in the test
harness. This isn't a big deal...IPTB's complexity is mostly around
the fact that its state needs to be saved to disk in between `iptb`
command invocations, and that it uses Go plugins to inject
functionality, neither of which are relevant here.

If we merge this, we'll have to live with bifurcated tests for a while
until they are all migrated. I'd recommend we self-enforce a rule
that, if we need to touch a sharness test, we migrate it and one more
test over to Go tests first. Then eventually we will have migrated
everything.
@guseggert guseggert force-pushed the feat/test-harness-cli branch from 3735602 to 564d260 Compare December 12, 2022 14:35
@guseggert guseggert enabled auto-merge (rebase) December 12, 2022 14:35
@guseggert guseggert merged commit 579175f into master Dec 12, 2022
@guseggert guseggert deleted the feat/test-harness-cli branch December 12, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants