Skip to content

Consider refactoring test/utils/httpserver.ts #3864

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

Closed
jsjoeio opened this issue Jul 27, 2021 · 1 comment
Closed

Consider refactoring test/utils/httpserver.ts #3864

jsjoeio opened this issue Jul 27, 2021 · 1 comment
Labels
chore Related to maintenance or clean up testing Anything related to testing
Milestone

Comments

@jsjoeio
Copy link
Contributor

jsjoeio commented Jul 27, 2021

While pair-programming on #3858 with @vapurrmaid, it wasn't super clear to us why test/utils/httpserver.ts was set it up as an abstraction on top of http.

We were trying to extend and use it for some new tests and had trouble following the code.

As noted during pair programming - Using the http and node-fetch libraries directly is easier (LoC and maintainability). I have concerns about the custom class used in testing that abstracts these.

Reasons to refactor

  • the interface will stay up-to-date (i.e. http and node-fetch) as opposed to code getting stale in our custom abstraction
  • easier to maintain

Reasons to leave as is

@code-asher brought up some good points saying if we were to refactor it and use http and node-fetch, we may end up duplicating a lot of code.

@jsjoeio jsjoeio added testing Anything related to testing chore Related to maintenance or clean up labels Jul 27, 2021
@jsjoeio jsjoeio added this to the Backlog milestone Jul 27, 2021
@jsjoeio jsjoeio added stale and removed stale labels Sep 3, 2021
@stale
Copy link

stale bot commented Mar 3, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no activity occurs in the next 5 days.

@stale stale bot added the stale label Mar 3, 2022
@stale stale bot closed this as completed Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to maintenance or clean up testing Anything related to testing
Projects
None yet
Development

No branches or pull requests

1 participant