chore(tests floresta-rpc): minor improvements for setup. #712
Conversation
jaoleal
force-pushed
the
dontexplodetestsifididntbuildflorestaplease
branch
from
be2f52f to
0d0a75c
Compare
| }; | ||
| } | ||
|
|
||
| // florestad not being found normally means that we dont want to run these tests |
There was a problem hiding this comment.
That's a strong assumption, in my case it always means I forgot to build. And I want to be notified about this.
Not convinced this change is a good idea
There was a problem hiding this comment.
That's a strong assumption.
Sorry for not being clear. These tests under floresta-rpc have setup dependency, it needs florestad to be already built, so, one that want to test floresta-rpc is actually testing florestad and other things together... which is not ideal IMHO, that would be the assumption that i made.
And I want to be notified about this.
Yes, just skipping the tests isnt ideal, it gives the wrong idea that it passed. WYT of feature-gating it ?
I can work a florestad-tests feature were we could place all tests that depend on the binary.
This would also solve the objetive that i had while working this code, not being prevented to run the tests on our database just because i forgot to build florestad, which, ideally, should be necessary each time that we run this test to avoid testing the incorrect code.
I know that we have opposite points here but we may find a way to achieve both behaviors ?
There was a problem hiding this comment.
Sorry for not being clear. These tests under floresta-rpc have setup dependency, it needs florestad to be already built, so, one that want to test floresta-rpc is actually testing florestad and other things together... which is not ideal IMHO, that would be the assumption that i made.
How would you test floresta-rpc without the server? Mocking a server sounds wasteful.
Yes, just skipping the tests isnt ideal, it gives the wrong idea that it passed. WYT of feature-gating it ?
You can skip floresta-rpc on cargo test.
I know that we have opposite points here but we may find a way to achieve both behaviors ?
just test should build florestad first? That's an easy fix
There was a problem hiding this comment.
just testshould buildflorestadfirst? That's an easy fix
Agree, we shouldn't skip tests silently, just make sure we have florestad built
3 participants
What is the purpose of this pull request?
Which crates are being modified?
Description and Notes
I often do a
cargo cleanto reclaim some space but always i forget to build florestad, which makes tests to explode.The first commit in this PR make the tests to be skipped if florestad isnt found.
The second adds support for getting binaries from
FLORESTA_TEMP_DIR.Contributor Checklist
just pcc(recommended but slower)just lint-features '-- -D warnings' && cargo test --releaseFinally, you are encouraged to sign all your commits (it proves authorship and guards against tampering—see How (and why) to sign Git commits and GitHub's guide to signing commits).