Skip to content

Initial whole-stack tests for the API Server #1259

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Oct 12, 2023
Merged

Initial whole-stack tests for the API Server #1259

merged 21 commits into from
Oct 12, 2023
+2,101 −336

Conversation

alfiedotwtf
Copy link
Contributor

Part of #1189

@alfiedotwtf alfiedotwtf marked this pull request as draft October 5, 2023 10:49
@TheQuantumPhysicist
Copy link
Contributor

This generally looks OK, besides the comments I made. It's still in draft, so I'm not sure what's left.

iljakuklic
iljakuklic reviewed Oct 5, 2023
View reviewed changes
Copy link
Contributor

@iljakuklic iljakuklic left a comment

Choose a reason for hiding this comment

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

Just a quick first pass

@alfiedotwtf
Copy link
Contributor Author

alfiedotwtf commented Oct 6, 2023 •
edited
Loading

It's still in draft, so I'm not sure what's left.

If you look at web-server/src/api/v1.rs, you'll see all the current endpoints. Currently only the chain/ and block/ have been done (with a hand full of TODOs within them), but there's transaction/, destination/ and pool/ that I'll work on today (and think about how to actually do destination/ given that the current values are placeholders rather than going to the database.

@iljakuklic
Copy link
Contributor

It's still in draft, so I'm not sure what's left.

If you look at web-server/src/api/v1.rs, you'll see all the current endpoints. Currently only the chain/ and block/ have been done (with a hand full of TODOs within them), but there's transaction/, destination/ and pool/ that I'll work on today (and think about how to actually do destination/ given that the current values are placeholders rather than going to the database.

Given the tests file is already over 1000 lines, would it make sense to break it down by endpoint tested? I guess the categorisation is not going to be 100% accurate since I can imagine tests that check a combination of endpoints but maybe better than a monolithic file.

@alfiedotwtf alfiedotwtf force-pushed the alfie/webserver-tests branch from e662369 to 3fd89cb Compare October 10, 2023 07:42
@alfiedotwtf
Copy link
Contributor Author

Given the tests file is already over 1000 lines, would it make sense to break it down by endpoint tested? I guess the categorisation is not going to be 100% accurate since I can imagine tests that check a combination of endpoints but maybe better than a monolithic file.

I use code folding in Emac a lot, and because I've put each set of tests within a separately named module within the file, when the test file is collapsed, it logically makes sense. Not as clear as spreading this out to multiple files though, but code folding does let you see the shape of the file and lets you drill down when needed.

@alfiedotwtf
Copy link
Contributor Author

I've added a few transaction() and transaction_merkle_path() tests, but there a many TODOs left in the comments.

This PR is getting quite big as it is, so maybe the TODOs can be done in a separate PR along with the Destination and Pool endpoint changes / tests.

@alfiedotwtf alfiedotwtf marked this pull request as ready for review October 10, 2023 11:58
TheQuantumPhysicist
TheQuantumPhysicist approved these changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@TheQuantumPhysicist TheQuantumPhysicist left a comment

Choose a reason for hiding this comment

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

Besides the TODOs, this is looking fine. I'm assuming these TODOs will be fixed next.

alfiedotwtf and others added 11 commits October 12, 2023 16:44
@alfiedotwtf
Add tcp listener convenience method to the API Server config
d444f87
@alfiedotwtf
Add clone to InMemory storage for testing
3f150b5
@alfiedotwtf
Allow the scanner state to release internal storage
4a016ef
@alfiedotwtf
Add more API Server web server error types
746180a
@alfiedotwtf
Cleanup to make API Server easier to test

Verified

This commit was signed with the committer’s verified signature.
kennytm
GPG key ID: FEF6C8051D0E013C
Verified
Learn about vigilant mode
ae42402
@alfiedotwtf
Fix conflicts

Verified

This commit was signed with the committer’s verified signature.
kennytm
GPG key ID: FEF6C8051D0E013C
Verified
Learn about vigilant mode
10f7c14
@TheQuantumPhysicist @alfiedotwtf
Get rid of BlockchainState's Option for storage and use an async clone

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
98af55d
@alfiedotwtf
Fix clippy

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
d2d78ea
@alfiedotwtf
Fix async blocks, and abort webserver on test exit

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
76a8e3b
@alfiedotwtf
Added new error types

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
9760523
@alfiedotwtf
Factored out repeated code

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
f3cba49
alfiedotwtf and others added 10 commits October 12, 2023 16:44
@alfiedotwtf
Factored out repeated code in tests

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
7f70261
@alfiedotwtf
Added tests for transaction endpoints

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3a9d19a
@alfiedotwtf
Fix do_checks

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3b9c524
@alfiedotwtf
API -> Api for consistency

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
131fb96
@alfiedotwtf
Add more Api Server tests

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
a990637
@TheQuantumPhysicist @alfiedotwtf
Minor renames

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
f941f4f
@TheQuantumPhysicist @alfiedotwtf
Split tests of API end points

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
4aae68c
@TheQuantumPhysicist @alfiedotwtf
Add comment about race condition
5ccb030
@TheQuantumPhysicist @alfiedotwtf
Add licenses
9a3c2c1
@TheQuantumPhysicist @alfiedotwtf
Rename a few crates in the api-server collection
9a5ef7a
@alfiedotwtf alfiedotwtf force-pushed the alfie/webserver-tests branch from 57b4967 to 9a5ef7a Compare October 12, 2023 05:48
@TheQuantumPhysicist TheQuantumPhysicist merged commit ccc4103 into master Oct 12, 2023
@TheQuantumPhysicist TheQuantumPhysicist deleted the alfie/webserver-tests branch October 12, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iljakuklic iljakuklic

@TheQuantumPhysicist TheQuantumPhysicist

@muursh muursh

@erubboli erubboli

@azarovh azarovh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@alfiedotwtf @TheQuantumPhysicist @iljakuklic