tests: create test framework to test rpc endpoints #792 #793

thomasmodeneis · 2020-04-18T14:17:15Z

Changes

create test framework to test rpc endpoints as client (follow JSON-RPC Interfaces for Polkadot Nodes)

Tests:

Checklist:

I have read CONTRIBUTING and CODE_OF_CONDUCT
I have provided as much information as possible and necessary
I have reviewed my own pull request before requesting a review
I have linked any relevant issues in my pull request comments
All integration tests and required coverage checks are passing

Issues:

related to Create test framework to test RPC endpoints #792

add it-stress task to Makefile export ServerResponse from package tests/rpc create temp config.toml for test

update stress_test.go with commented out init task that depends on #789

…as/feature-741-stress

…0","error":{"code":-32600,"message":"json: cannot unmarshal object into Go value of type [1]interface {}","data":{}},"id":1}

add all other pending rpc endpoint test cases (skip for now)

add config.toml add rpc tests to travis

fix init / startup

fix call to Get HighestBlockHash and related assert

…amer into thomas/feature-792

edwardmack

Looks good, I see that these test follow a similar pattern of setup test cases, start nodes, iterate test cases, tear down nodes. Would it make sense to create a util function that would handle start nodes, iterate and tear down, then just pass arrays of test cases to that utility?

Makefile

tests/spec/empty.go

tests/spec/rpc_01-system_test.go

noot

overall looks good, some comments

…es than planned but should be good now

…as/feature-792

…es than planned but should be good now

noot · 2020-04-24T17:08:07Z

Makefile

@@ -46,6 +46,10 @@ it-stress: build
 	@echo "  >  \033[32mRunning Integration Tests stress mode...\033[0m "
 	GOSSAMER_NODE_HOST=0.0.0.0 GOSSAMER_INTEGRATION_TEST_MODE=stress go test ./tests/stress/... -timeout=5m -p 1 -short -v

+it-rpc: build
+	@echo "  >  \033[32mRunning Integration Tests RPC Specs mode...\033[0m "
+	GOSSAMER_NODE_HOST=0.0.0.0 GOSSAMER_INTEGRATION_TEST_MODE=rpc_spec go test ./tests/rpc/... -timeout=5m -p 1 -short -v


GOSSAMER_INTEGRATION_TEST_MODE=rpc_spec

should be

GOSSAMER_INTEGRATION_TEST_MODE=rpc

So since the objective of naming it spec was to make a test suite, and you are not ok with calling it spec, but I really nice need to be something more than just rpc (since its supposed to be a test suite for the RPC endpoints that match against a test specification) I renamed the logs to RPC suite. Let me know if you have any objections.

noot · 2020-04-24T17:08:18Z

tests/rpc/rpc_00_test.go

+)
+
+func TestMain(m *testing.M) {
+	_, _ = fmt.Fprintln(os.Stdout, "Going to start RPC spec test")


remove spec

So since the objective of naming it spec was to make a test suite, and you are not ok with calling it spec, but I really nice need to be something more than just rpc (since its supposed to be a test suite for the RPC endpoints that match against a test specification) I renamed the logs to RPC suite. Let me know if you have any objections.

noot · 2020-04-24T17:08:31Z

tests/rpc/rpc_01-system_test.go

+)
+
+func TestSystemRPC(t *testing.T) {
+	if GOSSAMER_INTEGRATION_TEST_MODE != rpcSpec {


change rpcSpec to rpc

So since the objective of naming it spec was to make a test suite, and you are not ok with calling it spec, but I really nice need to be something more than just rpc (since its supposed to be a test suite for the RPC endpoints that match against a test specification) I renamed the logs to RPC suite. Let me know if you have any objections.

tests/rpc/rpc_01-system_test.go

noot · 2020-04-24T17:10:59Z

tests/rpc/rpc_01-system_test.go

+
+			t.Log("Will start assertion for ", "target", target, "type", reflect.TypeOf(target))
+
+			switch v := target.(type) {


shouldn't we switch on the test description? what if an rpc method returns the wrong type

The RPC method should never return the wrong type, if that is happening we are in deep trouble.

okay well this will panic if it returns the wrong type, is that the desired behaviour?

Why would it panic ? it just not going to match any type. If something else is going on with the endpoint it will be either captured at unit tests or on DecodeRPC method

There is also the require nil err just before the switch...if you happen to be able to prove the edge case you are mentioning I will be more than happy to enforce more rules. Right now it seems like a overkill...

looking at the DecodeRPC method, the switch statement seems very brittle and will need to be updated in case the RPC api changes. it would be better to pass in a target interface{} directly instead of targetType string and switching on it. maybe we could open an issue for that, or do you want to change it here?

Well, that one should be always updated. That is why this is going to be part of the CI.
The trick is that the DisallowUnknownFields will make it fail nicely if the endpoint return changes, so we will see the test failures beforehand.

tests/rpc/rpc_01-system_test.go

…es than planned

noot · 2020-04-24T20:57:10Z

tests/rpc/rpc_01-system_test.go

+			case *modules.SystemHealthResponse:
+				t.Log("Will assert SystemHealthResponse", "target", target)
+
+				require.Equal(t, test.expected.(modules.SystemHealthResponse).Health.IsSyncing, v.Health.IsSyncing)


this line could potentially panic - for example, say system_networkState returns a SystemHealthResponse for some reason (there are probably better examples in the other modules but this is these are the only ones implemented so far)
then, test.expected is SystemNetworkStateResponse, not SystemHealthResponse. then, the type conversion test.expected.(modules.SystemHealthResponse) will panic.

But it should not return that, I mean, why would the system_networkState return a type that belongs to the other endpoint ? that should be captured at the unit test level. This are integration tests.

We are assuming here, that the minimum tests have been already checked by proper unit testing. Again, if this is happening at the integration tests and we have a panic, that is a good thing to happen. At least we will know right away something is really broken.

okay I just think it's good practice to always do an a, ok := b.(mytype) to avoid potential panics, it also seems to make more sense to switch on the test description or method name instead of the expected type, but if you prefer it this way

sure, raised #815 to tackle this!

create test framework to test rpc endpoints as client (follow JSON-RPC Interfaces for Polkadot Nodes)

thomasmodeneis added 9 commits April 16, 2020 20:15

add funcs for start/check/stop N gossamer nodes

f97332f

add it-stress task to Makefile export ServerResponse from package tests/rpc create temp config.toml for test

fix lint

5b77868

add new request_utils.go with reusable code for RPC tasks

1900f0e

update stress_test.go with commented out init task that depends on #789

Merge branch 'development' of github.com:ChainSafe/gossamer into thom…

c135145

…as/feature-741-stress

update test to reuse new request_utils.go

2424040

update test to reuse common.go and fix lint

ebbd58f

update test to query chain_getHeader; its failing with {"jsonrpc":"2.…

7c03c9e

…0","error":{"code":-32600,"message":"json: cannot unmarshal object into Go value of type [1]interface {}","data":{}},"id":1}

refactor for more modularity on gossamer_utils.go

1c55210

add 3 implemented test cases

8d2c8ac

add all other pending rpc endpoint test cases (skip for now)

thomasmodeneis self-assigned this Apr 18, 2020

thomasmodeneis requested review from ryanchristo, edwardmack and noot April 18, 2020 14:17

add rpc config to env var

0f2c63e

add config.toml add rpc tests to travis

thomasmodeneis marked this pull request as ready for review April 20, 2020 20:52

thomasmodeneis added 6 commits April 21, 2020 16:11

add check for GOSSAMER_NODE_HOST on init

193b21b

fix init / startup

fix init/start

a11c40f

add log output to gossamer process (gossamer.log)

6dca980

fix call to Get HighestBlockHash and related assert

fix pr comment

d575d51

add missing file

bc2e873

fix lint

f5b18a4

ryanchristo changed the title ~~[WIP] create test framework to easily test all rpc endpoints as client (follow JSON-RPC Interfaces for Polkadot Nodes) #792~~ tests: create test framework to test rpc endpoints as client #792 Apr 21, 2020

ryanchristo changed the title ~~tests: create test framework to test rpc endpoints as client #792~~ tests: create test framework to test rpc endpoints #792 Apr 21, 2020

ryanchristo added the Status: Needs Review label Apr 21, 2020

thomasmodeneis added 2 commits April 21, 2020 21:23

go imports

9aef4fa

Merge branch 'thomas/feature-741-stress' of github.com:ChainSafe/goss…

2cf7050

…amer into thomas/feature-792

thomasmodeneis changed the title ~~tests: create test framework to test rpc endpoints #792~~ [wip] tests: create test framework to test rpc endpoints #792 Apr 21, 2020

fix lint

9d381c2

ryanchristo added Status: Needs Changes and removed Status: Needs Review labels Apr 21, 2020

thomasmodeneis added Status: Needs Review and removed Status: Needs Changes labels Apr 22, 2020

thomasmodeneis added 2 commits April 22, 2020 23:36

add copy, remove config.toml

b53866a

remove config.toml

7f47ec1

edwardmack approved these changes Apr 23, 2020

View reviewed changes

noot reviewed Apr 23, 2020

View reviewed changes

Makefile Outdated Show resolved Hide resolved

noot reviewed Apr 23, 2020

View reviewed changes

tests/spec/empty.go Outdated Show resolved Hide resolved

noot reviewed Apr 23, 2020

View reviewed changes

tests/spec/rpc_01-system_test.go Outdated Show resolved Hide resolved

noot reviewed Apr 23, 2020

View reviewed changes

tests/spec/rpc_01-system_test.go Outdated Show resolved Hide resolved

noot suggested changes Apr 23, 2020

View reviewed changes

thomasmodeneis added 4 commits April 23, 2020 23:28

moving from spec to rpc as requested on pr review

465aac0

moving from spec to rpc as requested on pr review required more chang…

0026cd2

…es than planned but should be good now

Merge branch 'development' of github.com:ChainSafe/gossamer into thom…

a328775

…as/feature-792

moving from spec to rpc as requested on pr review required more chang…

f485eea

…es than planned but should be good now

noot reviewed Apr 24, 2020

View reviewed changes

tests/rpc/rpc_01-system_test.go Show resolved Hide resolved

noot reviewed Apr 24, 2020

View reviewed changes

tests/rpc/rpc_01-system_test.go Show resolved Hide resolved

moving from spec to rpc as requested on pr review required more chang…

bbc859d

…es than planned

noot reviewed Apr 24, 2020

View reviewed changes

noot approved these changes Apr 24, 2020

View reviewed changes

thomasmodeneis added 3 commits April 28, 2020 17:37

Merge branch 'development' into thomas/feature-792

ae983e4

Merge branch 'development' into thomas/feature-792

e1ac677

bump codeclimate/diff-coverage that is stuck with not useful commit

8c01278

thomasmodeneis merged commit 8491803 into development Apr 29, 2020

thomasmodeneis deleted the thomas/feature-792 branch April 29, 2020 14:42

ryanchristo removed the Status: Needs Review label Apr 29, 2020

ryanchristo pushed a commit that referenced this pull request Jun 24, 2020

tests: create test framework to test rpc endpoints #792 (#793)

b6fb2ff

create test framework to test rpc endpoints as client (follow JSON-RPC Interfaces for Polkadot Nodes)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: create test framework to test rpc endpoints #792 #793

tests: create test framework to test rpc endpoints #792 #793

thomasmodeneis commented Apr 18, 2020 •

edited

Loading

edwardmack left a comment

noot left a comment

noot Apr 24, 2020

thomasmodeneis Apr 24, 2020

noot Apr 24, 2020

thomasmodeneis Apr 24, 2020 •

edited

Loading

noot Apr 24, 2020

thomasmodeneis Apr 24, 2020 •

edited

Loading

noot Apr 24, 2020

thomasmodeneis Apr 24, 2020

noot Apr 24, 2020

thomasmodeneis Apr 24, 2020

thomasmodeneis Apr 24, 2020

noot Apr 24, 2020 •

edited

Loading

thomasmodeneis Apr 28, 2020

noot Apr 24, 2020

thomasmodeneis Apr 24, 2020

thomasmodeneis Apr 24, 2020

noot Apr 24, 2020

thomasmodeneis Apr 28, 2020


		t.Log("Will start assertion for ", "target", target, "type", reflect.TypeOf(target))

		switch v := target.(type) {

tests: create test framework to test rpc endpoints #792 #793

tests: create test framework to test rpc endpoints #792 #793

Conversation

thomasmodeneis commented Apr 18, 2020 • edited Loading

Changes

Tests:

Checklist:

Issues:

edwardmack left a comment

Choose a reason for hiding this comment

noot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasmodeneis Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasmodeneis Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noot Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasmodeneis commented Apr 18, 2020 •

edited

Loading

thomasmodeneis Apr 24, 2020 •

edited

Loading

thomasmodeneis Apr 24, 2020 •

edited

Loading

noot Apr 24, 2020 •

edited

Loading