Skip to content

test: remove unnecessary urlToHttpOptions test - #47002

Closed
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:fix-remove-unnecessary-test
Closed

test: remove unnecessary urlToHttpOptions test#47002
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:fix-remove-unnecessary-test

Conversation

@anonrig

Copy link
Copy Markdown
Member

The following test is not needed anymore. We removed the symbol in URL, and the following test is unnecessary. Reference: #46904

We don't even check for isURL on urlToHttpOptions, but the parameter of urlToHttpOptions is expected to be URL.

Referencing the discussion with @aduh95 on #46989

cc @nodejs/url

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Mar 7, 2023
@anonriganonrig added the url Issues and PRs related to the legacy built-in url module. label Mar 7, 2023

@aduh95aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don’t think it’s a good reason to remove the test.

@anonriganonrig closed this Mar 8, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.testIssues and PRs related to the tests.urlIssues and PRs related to the legacy built-in url module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@anonrig@aduh95@nodejs-github-bot