Skip to content

feature: Revise integration tests #3420

Closed
@ntnn

Description

@ntnn

Feature Description

Revise integration tests to expose the underlying pkg/server.Server.

Things like the DynamicRESTMapper cannot be tested properly at the moment as the structure is not exposed to any of the tests.

When adding test/integration I originally had a separate setup function but adjusted sdk/testing instead as this was already doing most of what was needed to start and stop the server.

However proper integration tests - like for DynamicRESTMapper - would require exposing the underlying server struct.

Proposed Solution

The easiest solution:
Restructure the framework to allow for runner to optionally return the server struct and store this in kcpServer. For external servers this can be simply nil.
The RunningServer interface could then have a method to return this server and it's up to the user to use this correctly.

The slightly harder solution:
Rewrite test/integration/framework to expose the underlying server struct.

The harder solution:
Redesign sdk/testing, test/e2e/framework and test/integration/framework with all this in mind.

Alternative Solutions

No response

Want to contribute?

  • I would like to work on this issue.

Additional Context

Personally I'm not a big fan of the easiest solution because sdk/testing... is already messy as is.
Then again this would probably just be a <10sloc changecitation needed.

Then again I'm also not a fan of rewriting test/integration/framework because it would probably duplicate a good chunk of code from sdk/testing - which is what I wanted to avoid intially by using test/e2e/framework/inprocess and sdk/testing.

Metadata

Metadata

Assignees

Labels

kind/featureCategorizes issue or PR as related to a new feature.¯\_(ツ)_/¯¯\\\_(ツ)_/¯

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions