Skip to content

Issue 1362#354

Merged
ondrejmirtes merged 4 commits intophpstan:masterfrom
jfreixa:issue-1362
Oct 23, 2020
Merged

Issue 1362#354
ondrejmirtes merged 4 commits intophpstan:masterfrom
jfreixa:issue-1362

Conversation

@jfreixa
Copy link
Copy Markdown

@jfreixa jfreixa commented Oct 23, 2020


public function testInvalidEncoding(): void
{
$this->assertIsString((new ConstantStringType(file_get_contents('invalidUtf8Characters.txt', true)))->describe(VerbosityLevel::value()));
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I cannot use assertEquals or assertSame due I don't have the same string. Which assert would you use in this situation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

assertSame with the exact string produced in the test


public function testInvalidEncoding(): void
{
$this->assertIsString((new ConstantStringType(file_get_contents('invalidUtf8Characters.txt', true)))->describe(VerbosityLevel::value()));
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Is it ok to use file_get_contents in this test or I should move it to a different test?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please produce invalid UTF-8 by using \x00 etc. characters in the source code directly.


public function testInvalidEncoding(): void
{
$this->assertIsString((new ConstantStringType(file_get_contents('invalidUtf8Characters.txt', true)))->describe(VerbosityLevel::value()));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

assertSame with the exact string produced in the test


public function testInvalidEncoding(): void
{
$this->assertIsString((new ConstantStringType(file_get_contents('invalidUtf8Characters.txt', true)))->describe(VerbosityLevel::value()));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please produce invalid UTF-8 by using \x00 etc. characters in the source code directly.

try {
$truncatedValue = \Nette\Utils\Strings::truncate($this->value, self::DESCRIBE_LIMIT);
} catch (\Nette\Utils\RegexpException $e) {
$truncatedValue = substr($this->value, 0, self::DESCRIBE_LIMIT);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're also missing the ellipsis here in case the string is longer than self::DESCRIBE_LIMIT.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I added the ellipsis always because the exception is not thrown when the string is shorter than self::DESCRIBE_LIMIT.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a test that short invalid UTF-8 is not truncated and ellipsis not added.

@jfreixa jfreixa requested a review from ondrejmirtes October 23, 2020 09:10
@ondrejmirtes ondrejmirtes merged commit 0dee4c3 into phpstan:master Oct 23, 2020
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

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.

2 participants