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

Improve test coverage #153

Merged
merged 6 commits into from
May 21, 2024
Merged

Improve test coverage #153

merged 6 commits into from
May 21, 2024

Conversation

Gesma94
Copy link
Contributor

@Gesma94 Gesma94 commented Apr 29, 2024

To help obtain 100% coverage from #126, I've added few tests. Also, few comments on some files that are still not 100% covered:

  • decodeText.js still misses line 97 and 98. Though, I'm not sure they will ever be hit:

    1. this keyword at line 96 is used, but the method is an arrow function which is called via bind(charset) from line 42; though, I'm afraid that, being an arrow function, the scope of the this inside of it will not be the passed value of charset
    2. Supposing we change from an arrow function to a function, I don't see how the if statement at line 96 will pass: we ask if the textDecoders has the given charset, where textDecoders includes utf-8 and utf8. But, if the charset passed at line 42 is one of the two values, then another branch of the switch statement is taken.
  • urlencoded.js still misses line 118 and 164, but again, I'm not sure that they will ever be hit.

    • In order to not enter the if statement at line 118, p needs to be higher than len, and the only way I figured out is the loop for at line 58, where inside at line 59 we have if (!this._checkingBytes) { ++p }
    • In order to enter this last if, this._checkingBytes must be false, which happens at line 112, which is inside an else if (this._hitLimit)
    • Thus, I need this._hitlimit equals to true, but.. If that happens, since that if else statement is before the last else at line 117 which leads to line 118, the line 118 cannot be reached with p being greater or equals to len
    • For what concern line 164, the reasoning is basically the same, since the only difference is that, for line 164, we are working on the parameter value, while for line 118 we are working for the parameter name

Of course, I may be wrong for both files: I tried to go through all the code and understanding it, but maybe I'm missing some pieces.

Checklist

@gurgunday gurgunday requested a review from Uzlopak May 15, 2024 22:22
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

lgtm

@gurgunday gurgunday merged commit 8080532 into fastify:master May 21, 2024
18 checks passed
@gurgunday gurgunday mentioned this pull request May 21, 2024
2 tasks
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.

None yet

2 participants