-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
Updates to Bootstrap 5 and adds theme support for code coverage. #1036
base: main
Are you sure you want to change the base?
Updates to Bootstrap 5 and adds theme support for code coverage. #1036
Conversation
…pdate for test EndToEndTest::testPathCoverageForBankAccountTest
|
||
</tbody> | ||
</table> | ||
%a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is correct way to fix this test by adding the footer.
… for BankAccount.php_path.html.
Should I create another Pull Request, in PHPUnit, to add a theme variable to phpunit.xsd? <xs:complexType name="coverageReportHtmlType">
<xs:attribute name="outputDirectory" type="xs:anyURI" use="required"/>
<xs:attribute name="lowUpperBound" type="xs:integer" default="50"/>
<xs:attribute name="highLowerBound" type="xs:integer" default="90"/>
<xs:attribute name="colorSuccessLow" type="xs:string" default="#dff0d8"/>
<xs:attribute name="colorSuccessMedium" type="xs:string" default="#c3e3b5"/>
<xs:attribute name="colorSuccessHigh" type="xs:string" default="#99cb84"/>
<xs:attribute name="colorWarning" type="xs:string" default="#fcf8e3"/>
<xs:attribute name="colorDanger" type="xs:string" default="#f2dede"/>
<xs:attribute name="customCssFile" type="xs:string"/>
<xs:attribute name="theme" type="xs:string" default=""/>
</xs:complexType> The colors for low, medium, high, warning, and danger can be controlled by setting the CSS Variables: .table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: {{success-low}};
/* background-color: rgb(from var(--bs-success) r g b / 0.25); */
}
The Bootstrap CSS Variables may be overridden in a theme defined with the existing:
NOTE: The CSS rules defined in .table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: var(--bs-teal);
} I tested passing both overriding with the customCssFile and in overriding the colors with CSS variables in the phpunit.xml configuration as well and both work fine.
colors<html
outputDirectory="output/html"
lowUpperBound="50"
highLowerBound="90"
customCssFile="./phpunit-themes.css"
colorSuccessLow="rgb(from var(--bs-purple) r g b / 0.75)"
colorSuccessMedium="rgb(from var(--bs-blue) r g b / 0.55)"
colorSuccessHigh="rgb(from var(--bs-teal) r g b / 0.25)"
colorWarning="rgb(from var(--bs-warning) r g b / 0.25)"
colorDanger="rgb(from var(--bs-danger) r g b / 0.25)"
theme="dark" /> And with these custom CSS rules:.table tbody tr.covered-by-medium-tests td, li.covered-by-medium-tests {
background-color: purple;
}
.table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: aquamarine;
}
.table tbody tr.danger td, .table tbody td.danger, li.danger, span.danger {
background-color: wheat;
}
.table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: aquamarine;
}
.table tbody tr.covered-by-medium-tests td, li.covered-by-medium-tests {
background-color: purple;
}
.table tbody tr.covered-by-small-tests td, li.covered-by-small-tests {
background-color: var(--bs-teal);
}
.table tbody tr.warning td, .table tbody td.warning, li.warning, span.warning {
background-color: var(--bs-yellow);
}
.table tbody tr.danger td, .table tbody td.danger, li.danger, span.danger {
background-color: crimson;
}
.covered-by-small-tests, tr.covered-by-small-tests td {
background-color: var(--bs-teal);
}
.covered-by-medium-tests, tr.covered-by-medium-tests td {
background-color: purple;
}
.covered-by-large-tests, tr.covered-by-large-tests td {
background-color: aquamarine;
}
.not-covered, tr.not-covered td {
background-color: crimson;
}
.not-coverable, tr.not-coverable td {
background-color: var(--bs-yellow);
} |
I wonder whether we need this new |
The main reason why the <html
outputDirectory="output/html"
lowUpperBound="50"
highLowerBound="90"
theme="dark" />
I added some more information on the changes needed for the |
Please split this into two pull requests: one for the Bootstrap update and one for the functional changes. |
Here are the Bootstrap-only changes, for FYI:
final class Colors
{
private readonly string $successLow;
private readonly string $successMedium;
private readonly string $successHigh;
private readonly string $warning;
private readonly string $danger;
public static function default(): self
{
return new self(
'rgb(from var(--bs-success) r g b / 0.1)',
'rgb(from var(--bs-success) r g b / 0.33)',
'rgb(from var(--bs-success) r g b / 0.67)',
'rgb(from var(--bs-warning) r g b / 0.1)',
'rgb(from var(--bs-danger) r g b / 0.1)',
);
}
<html lang="en" data-bs-theme="dark"> |
FYI: I just tried running the tests, my installation is on PHPUnit 11.3.0 and I did not see the whitespace error.
|
My last test (#1037 (comment)) was with the other PR (#1037). Can you please test with that, too? Thanks! |
Changes
Assets
I used these from jsdeliver and Cloudflare:
Theme support
Bootstrap 5.3 comes with dark and light mode support.
The
<html>
tag of each top level webpage gets a new element:theme
variable in code coverage section:For example: src/Report/Html/Renderer/Template/directory.html.dist
This custom theme file provides some simple examples, the colors are not perfect, but good enough to test the code.
I put these themes into
customCssFile="./phpunit-themes.css"
Example phpunit-themes.css for customCssFile
phpunit.xml changes
Proposed theme variable:
CSS Variables in style.css
I had to slightly adjust the existing CSS rules, such as adding a
td
to increase override priority from the Bootstrap 5 rules.See src/Report/Html/Renderer/Template/css/style.css
These new default rules should be compatible across other themes.
Here are few I updated, but did not finalize:
Dev Notes
Testing themes
Here is a screenshot of testing the example themes in:
phpunit-themes.css
You can inspect the DOM to manually change
Tech Debt
There are a few other things that are a bit out of date.
NOTE: I think these fixes are beyond the scope of this PR and I would not mind tackling those after this:
align="center"
.