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

Get rid of unused tests #693

Closed
evan-forbes opened this issue Sep 8, 2022 · 4 comments · Fixed by #692 or #697
Closed

Get rid of unused tests #693

evan-forbes opened this issue Sep 8, 2022 · 4 comments · Fixed by #692 or #697
Assignees
Labels
chore optional label for items that follow the `chore` conventional commit

Comments

@evan-forbes
Copy link
Member

There are simulation tests that aren't testing anything, and are remnants of starport. While we certainly want equivalent tests, we will not be able to rely on the current simulation package or the testapp given that most of the application is stateless and those tools were not designed for that.

https://github.com/celestiaorg/celestia-app/blob/main/contrib/devtools/sims.mk
https://github.com/celestiaorg/celestia-app/blob/main/.github/workflows/release-sims.yml
https://github.com/celestiaorg/celestia-app/blob/main/.github/workflows/sims.yml

I'm also curious as to why these tests didn't start failing until recently

@rootulp
Copy link
Collaborator

rootulp commented Sep 8, 2022

@evan-forbes
Copy link
Member Author

sims failed on my branch for a known reason: https://github.com/celestiaorg/celestia-app/actions/runs/3012237262

what was this reason?

sims passes on recent runs main: https://github.com/celestiaorg/celestia-app/actions/workflows/sims.yml?query=branch%3Amain

yeah! the thing that urks me is that I don't know why its passing in the first place lolol

sims failed on https://github.com/celestiaorg/celestia-app/runs/8242496184?check_suite_focus=true
the -Enabled flag looks suspicious ref
likely not worth investigating / fixing if we don't need sims.mk

yeah, I tried to remove flags, and it just kept giving the same error but the next flag/command

@rootulp
Copy link
Collaborator

rootulp commented Sep 8, 2022

what was this reason?

go build fails because this draft PR depends on celestia-core changes. The draft PR doesn't modify go.mod because the celestia-core changes in this PR haven't been merged or released.

rootulp added a commit to rootulp/celestia-app that referenced this issue Sep 12, 2022
@rootulp rootulp self-assigned this Sep 12, 2022
@rootulp rootulp added the chore optional label for items that follow the `chore` conventional commit label Sep 12, 2022
@rootulp rootulp removed their assignment Sep 12, 2022
@rootulp
Copy link
Collaborator

rootulp commented Sep 12, 2022

Research into sims

@rootulp rootulp self-assigned this Sep 12, 2022
rootulp added a commit that referenced this issue Sep 12, 2022
* chore: remove sims

resolves #693

* chore: delete sims from root Makefile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore optional label for items that follow the `chore` conventional commit
Projects
None yet
2 participants