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

Boosted http-client code coverage to 100% + minor updates #234

Merged
merged 3 commits into from
Apr 5, 2024

Conversation

thehenrytsai
Copy link
Member

  • Boosted http-client to have 100% code coverage
  • Removed seemingly unnecessary explicit calls of setPrototypeOf()
  • Added html reporter for code coverage for local detailed report

Copy link

changeset-bot bot commented Apr 5, 2024

⚠️ No Changeset found

Latest commit: ee0cc18

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Apr 5, 2024

TBDocs Report

🛑 Errors: 0
⚠️ Warnings: 3

@tbdex/protocol

  • Project entry file: packages/protocol/src/main.ts

@tbdex/http-client

  • Project entry file: packages/http-client/src/main.ts

@tbdex/http-server

  • Project entry file: packages/http-server/src/main.ts
📄 File: packages/http-server/src/http-server.ts
⚠️ extractor:ae-unresolved-link: The @link reference could not be resolved: The package "@tbdex/http-server" does not have an export "InMemoryOfferingsApi" #L49
⚠️ extractor:ae-unresolved-link: The @link reference could not be resolved: The package "@tbdex/http-server" does not have an export "InMemoryExchangesApi" #L49
⚠️ extractor:ae-unresolved-link: The @link reference could not be resolved: The package "@tbdex/http-server" does not have an export "InMemoryBalancesApi" #L49

TBDocs Report Updated at 2024-04-05T04:06:17Z ee0cc18

Comment on lines 492 to 494
const bob = await DidJwk.create()
const bobRfq = await DevTools.createRfq({ sender: bob })
await bobRfq.sign(bob)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe what would make more sense naming wise is to say aliceRfq1 and aliceRfq2, since the GET /exchanges call would be returning all exchanges for a given did? so maybe something like

Suggested change
const bob = await DidJwk.create()
const bobRfq = await DevTools.createRfq({ sender: bob })
await bobRfq.sign(bob)
const aliceRfq2 = await DevTools.createRfq({ sender: alice })
await aliceRfq2.sign(alice)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thanks @jiyoontbd for the knowledge, makes sense, didn't realize the did property was meant to be for the caller, I've updated that too, let me know if I misunderstood again!

Comment on lines 489 to 490
const aliceRfq = await DevTools.createRfq({ sender: alice })
await aliceRfq.sign(alice)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const aliceRfq = await DevTools.createRfq({ sender: alice })
await aliceRfq.sign(alice)
const aliceRfq1 = await DevTools.createRfq({ sender: alice })
await aliceRfq1.sign(alice)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2024

Codecov Report

Merging #234 (ee0cc18) into main (9a91ef8) will increase coverage by 0.84%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
+ Coverage   93.92%   94.77%   +0.84%     
==========================================
  Files          42       42              
  Lines        3590     3541      -49     
  Branches      402      390      -12     
==========================================
- Hits         3372     3356      -16     
+ Misses        218      185      -33     
Components Coverage Δ
protocol 94.90% <ø> (ø)
http-client 100.00% <100.00%> (+4.72%) ⬆️
http-server 91.07% <ø> (+0.19%) ⬆️

@thehenrytsai thehenrytsai merged commit 5d690ca into main Apr 5, 2024
8 checks passed
@thehenrytsai thehenrytsai deleted the henrytsai/http-client-full-coverage branch April 5, 2024 04:17
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

Successfully merging this pull request may close these issues.

4 participants