Skip to content

feat: integration test helpers#15556

Merged
julienrbrt merged 23 commits into
mainfrom
julien/integration
Mar 31, 2023
Merged

feat: integration test helpers#15556
julienrbrt merged 23 commits into
mainfrom
julien/integration

Conversation

@julienrbrt
Copy link
Copy Markdown
Contributor

@julienrbrt julienrbrt commented Mar 27, 2023

Description

Closes: #14676

I have not pushed everything yet: (#15556 (comment))

  • refactor tests/integration (here I am doing staking and evidence)
  • keep test coverage the same (I am currently moving tests around to keep the area tested)

I am as well exploring a way to register the msg server and query server directly when creating a integration app.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@julienrbrt julienrbrt changed the title feat: integration test framework feat: integration test framework (tentative implementation) Mar 27, 2023
@julienrbrt
Copy link
Copy Markdown
Contributor Author

julienrbrt commented Mar 28, 2023

Opening already, so possibly we can migrate module in follow-ups.
I'm still continuing the work I mentioned in the issue; however, I'll open PRs against this branch.

@julienrbrt julienrbrt marked this pull request as ready for review March 28, 2023 14:06
@julienrbrt julienrbrt requested review from a team and likhita-809 March 28, 2023 14:06
@ghost ghost requested review from a team and aaronc and removed request for a team March 28, 2023 14:06
@julienrbrt julienrbrt requested review from mark-rushakoff and tac0turtle and removed request for aaronc March 28, 2023 14:07
@julienrbrt julienrbrt changed the title feat: integration test framework (tentative implementation) feat: integration test helpers (tentative implementation) Mar 28, 2023
@kocubinski kocubinski self-assigned this Mar 28, 2023
Comment thread docs/docs/building-modules/11-structure.md Outdated
:::note
You can as well use the `AppConfig` `configurator` for creating an `AppConfig` [inline](https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/x/slashing/app_test.go#L54-L62). There no difference between those two ways, use whichever you prefer.
:::
Integration tests interact with the tested module via the defined `Msg` and `Query` services. The result of the test can be verified by checking the state of the application, by checking the emitted events or the response. It is adviced to combine two of these methods to verify the result of the test.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

will we document how to set things up here or elsewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here, as they are for testing modules integrations.

Copy link
Copy Markdown
Contributor

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

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

I wasn't able to read all the way through this before my end of day, but I did leave two comments. I can take another look tomorrow.

Comment thread tests/integration/slashing/integration.go Outdated
Comment thread testutil/integration/router.go Outdated
Copy link
Copy Markdown
Contributor

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

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

I think I follow what's going on here now. Structure and organization looks fine overall.

Let's change the test labeled example to an example test so it shows up at pkg.go.dev, and there were a few other small details I called out to help consumers follow intent here.

Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/router.go Outdated
Comment thread testutil/integration/router.go Outdated
Comment thread testutil/integration/router.go
Comment thread testutil/integration/router.go Outdated
Comment thread testutil/integration/router.go
Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/router.go Outdated
@julienrbrt
Copy link
Copy Markdown
Contributor Author

Thank you for the great feedback @mark-rushakoff! I've added your suggestions.

@julienrbrt julienrbrt changed the title feat: integration test helpers (tentative implementation) feat: integration test helpers Mar 31, 2023
@julienrbrt julienrbrt enabled auto-merge (squash) March 31, 2023 15:40
Copy link
Copy Markdown
Contributor

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

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

I didn't read the markdown changes closely, nor did I look closely at the changes in tests/integration/slashing/keeper/keeper_test.go. But the additions to testutil/integration look fine to me.

I noted a couple other small typos here, but otherwise I think this can be merged.

Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/router.go Outdated
Comment thread testutil/integration/router.go Outdated
@julienrbrt julienrbrt disabled auto-merge March 31, 2023 15:41
@julienrbrt julienrbrt enabled auto-merge (squash) March 31, 2023 15:44
Comment thread docs/docs/building-modules/16-testing.md Outdated
Copy link
Copy Markdown
Contributor

@likhita-809 likhita-809 left a comment

Choose a reason for hiding this comment

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

nice work julien

Comment thread docs/docs/building-modules/16-testing.md Outdated
Comment thread testutil/integration/doc.go Outdated
Comment thread testutil/integration/example_test.go Outdated
Comment thread testutil/integration/router.go Outdated
@julienrbrt julienrbrt disabled auto-merge March 31, 2023 17:13
@julienrbrt julienrbrt enabled auto-merge (squash) March 31, 2023 17:17
@julienrbrt julienrbrt merged commit aeaa301 into main Mar 31, 2023
@julienrbrt julienrbrt deleted the julien/integration branch March 31, 2023 18:02
Copy link
Copy Markdown
Member

@aaronc aaronc left a comment

Choose a reason for hiding this comment

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

Good to see this work happening. Would be nice to align it with the direction core API is going. I think we also need ways to advance blocks so that begin and end blockers get called

Comment thread testutil/integration/router.go
return app.ctx
}

func (app *App) QueryHelper() *baseapp.QueryServiceTestHelper {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why don't we just return grpc.ClientConnInterface?

@julienrbrt
Copy link
Copy Markdown
Contributor Author

Good to see this work happening. Would be nice to align it with the direction core API is going. I think we also need ways to advance blocks so that begin and end blockers get called

Right, Marko mentioned that too. I'll open a PR about the last comment.

About Core API I don't think we can update it here just yet.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Research an integration framework and create PoC

6 participants