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

Problem: TSN isn't properly tested in CI #334

Closed
Tracked by #415
outerlook opened this issue Jun 26, 2024 · 13 comments
Closed
Tracked by #415

Problem: TSN isn't properly tested in CI #334

outerlook opened this issue Jun 26, 2024 · 13 comments

Comments

@outerlook
Copy link
Contributor

I know this issue isn't a priority, but it's nice to register

Our tests are manually executed during development with the markdown scripts.

We should create an integration test for those and include at CI

@brennanjl
Copy link
Collaborator

Our team is actually just finishing our testing framework (kwilteam/kwil-db#856). We are planning to backport to the preview branch, so that your team can use it. Trying to get it there by the end of the week.

@zolotokrylin
Copy link
Contributor

@brennanjl did it happen?

@brennanjl
Copy link
Collaborator

@zolotokrylin yes, it got merged a few weeks ago, however the docs aren't up yet. Will get the documentation up today.

@brennanjl
Copy link
Collaborator

@outerlook we have published docs for testing here: https://prerelease.kwil.com/docs/kuneiform/testing/intro/

You will have to either build kwil-cli from the preview branch, or import the testing SDK from the preview branch:

go get github.com/kwilteam/kwil-db/testing@preview

@outerlook
Copy link
Contributor Author

outerlook commented Jul 16, 2024

feedback relevant to tsn-sdk and tsn-data-provider tooling:

It would be powerful to be able to target this temporary database with a kwil client... Or we can already? I imagine it's because the framework skips kwild and directly connects to the test DB right?

The intention here is to test our SDK (that leverages kwil client) using the same temporary environment

I'll try using the options Conn and UseTestContainer=false to achieve that. Does this sound like a reasonable request for backlogging?

Edit: I see that there are many pitfalls here that can make it too complex for now


  • Being able to set a different name for a test contract would be nice. we deploy multiple streams with the same name in kf. Names just change when deploying
  • use custom deployer so we can reuse in non test container

@brennanjl
Copy link
Collaborator

Being able to set a different name for a test contract would be nice. we deploy multiple streams with the same name in kf. Names just change when deploying

If you are deploying with Kwil-cli, there is a --name flag that can do this. If you are using an SDK, you can just manually edit the JSON's name field after the schema is parsed.

use custom deployer so we can reuse in non test container

Can you elaborate on this? I'm not sure if I follow.

It would be powerful to be able to target this temporary database with a kwil client... Or we can already?

We cannot, for exactly the reason you described. The test works by running Kwil's execution engine against Postgres. It does not, however, expose an HTTP server. Kwil's HTTP server only accepts blockchain transactions, and similarly the client only sends blockchain transactions.

I can see why this would be desireable though. You essentially want a way to test any other logic against a live database without the need to wait for consensus, right?

@outerlook
Copy link
Contributor Author

outerlook commented Jul 17, 2024

If you are deploying with Kwil-cli, there is a --name flag that can do this

I mean, in the testing framework 😆

Here, we can set what are the schema files but not the schema name. Maybe we could also instead provide parsed schemas to solve that? I know the workaround of providing plain text schemas instead of files, so we can search & replace the name manually. Example usage: we want to use 2 primitive contracts and 1 composed, then we need to use that, but that workaround is available

use custom deployer so we can reuse in non test container

Can you elaborate on this? I'm not sure if I follow.

I was thinking about this workaround of specifying a real kwil database connection so I could reuse the setup&teardown features, but on a real kwil database. But for that to work, I think I would need to able to specify a different database deployer, instead of the hardcoded one

You essentially want a way to test any other logic against a live database without the need to wait for consensus, right?

I understand your points. And I think your description is accurate. That would for sure help. The setup&teardown ergonomy is also great there, but we can handle that step based on your framework

@zolotokrylin
Copy link
Contributor

What's a parent Goal here, please?
Please help to set it either as a sub-issue (Problem) of a specific Goal or convert it into a Goal.

Thank you.

@outerlook
Copy link
Contributor Author

Hey @zolotokrylin, I initially raised this issue just to get it out of my head, as I felt its pain from time to time. When that kind of situation happens, should I create a goal instead? What is the process?

If we should include this in a phase, I'd put some goal such as "Improving TSN tests reliability" which could include the tasks

  • create X tests
  • make tests run in CI
  • etc

@zolotokrylin
Copy link
Contributor

@outerlook All good. You've done everything right by pitting it in GitHub.

You can create a goal if it makes sense to be done in isolation from everything else. If it's a problem that belongs to a bigger goal, then it needs to be attached to a relevant goal as a problem that stops you from achieving it.

@outerlook
Copy link
Contributor Author

I'll set it a goal in isolation soon then, as it does not block other goals, just improves the quality/rework

@outerlook
Copy link
Contributor Author

Log relevant to the probable reliability goal

We need better observability for our debugs, even more when sharing with kwil team: store logs (easiest now, just cloudwatch), observe storage usage from nodes, indexer, etc

(I'll still open the goal)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants