test: cover the remaining long-tail branches - #866
Merged
Conversation
Error paths and unused accessors across five classes, none of which had a test for the
branch in question.
DataGridBase: the three independent catch sites for a missing template — header, pager
and row — each reached separately, so a fix that only guarded one would still fail two
of these.
FileHandler: the lock and unlock failure paths, and delete's failed unlink. The lock ones
need a custom stream wrapper: real flock() has no permission-based failure and the code
calls it without LOCK_NB, so a genuine conflict would block rather than return false.
The unlink one swaps a directory in for the file after opening it, which fails EISDIR
regardless of who is running the suite.
Serde: both JSON error paths — NAN defeats json_encode with JSON_THROW_ON_ERROR, and
truncated input defeats json_decode.
JsonMessage and AccountHistoryView: accessors that were never read.
Three branches are deliberately left uncovered, each established by measurement:
- XmlExport::createDocument() only calls `new DOMDocument('1.0', 'UTF-8')` and
createElement('Root') on literals, in a private method called once from the constructor
of a final class. No input or seam can make it throw.
- FileHandler's `fgetcsv() === false` guard: SplFileObject::fgetcsv() returns `[null]` on
exhaustion and false only at true EOF, which the loop condition has already excluded.
- Functions.php's `if (!defined('MODULES_PATH'))` inside initModule(): the constant is
defined unconditionally as the first executable line of the same file.
Also found while writing the DataGrid tests, and left for its own change:
DataGridBase::getDataHeaderTemplate() is declared `: string` while its backing property is
`?string` and is only assigned inside the try block that the missing-template case skips.
Calling it in that state is a TypeError. Its three siblings are all declared `?string`.
The test here asserts the setter's behaviour and does not call that getter, so it does not
depend on which way that is resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Error paths and unused accessors across five classes, none of which had a test for the
branch in question.
DataGridBase: the three independent catch sites for a missing template — header, pager
and row — each reached separately, so a fix that only guarded one would still fail two
of these.
FileHandler: the lock and unlock failure paths, and delete's failed unlink. The lock ones
need a custom stream wrapper: real flock() has no permission-based failure and the code
calls it without LOCK_NB, so a genuine conflict would block rather than return false.
The unlink one swaps a directory in for the file after opening it, which fails EISDIR
regardless of who is running the suite.
Serde: both JSON error paths — NAN defeats json_encode with JSON_THROW_ON_ERROR, and
truncated input defeats json_decode.
JsonMessage and AccountHistoryView: accessors that were never read.
Three branches are deliberately left uncovered, each established by measurement:
new DOMDocument('1.0', 'UTF-8')andcreateElement('Root') on literals, in a private method called once from the constructor
of a final class. No input or seam can make it throw.
fgetcsv() === falseguard: SplFileObject::fgetcsv() returns[null]onexhaustion and false only at true EOF, which the loop condition has already excluded.
if (!defined('MODULES_PATH'))inside initModule(): the constant isdefined unconditionally as the first executable line of the same file.
Also found while writing the DataGrid tests, and left for its own change:
DataGridBase::getDataHeaderTemplate() is declared
: stringwhile its backing property is?stringand is only assigned inside the try block that the missing-template case skips.Calling it in that state is a TypeError. Its three siblings are all declared
?string.The test here asserts the setter's behaviour and does not call that getter, so it does not
depend on which way that is resolved.