Tests: Add Tests for Printing. by mikeller · Pull Request #4736 · subsurface/subsurface

mikeller · 2026-03-01T12:11:28Z

Describe the pull request:

Pull request long description:

Add tests to verify that the profile substitution works for the default
templates and a number of custom templates with different html formatting.

Changes made:

Related issues:

Additional information:

Documentation change:

Mentions:

Copilot

Pull request overview

Adds a dedicated Qt test target to validate printing-template behavior (HTML substitution and dive-profile placeholder handling) in the desktop build, along with several template fixtures used by the tests.

Changes:

Introduces TestPrinting (QTest) to verify dive.id substitution in exported HTML and that profile placeholders are recognized during render.
Updates tests/CMakeLists.txt to build/run the new test (desktop-only, printing enabled) and to run Linux tests with QT_QPA_PLATFORM=offscreen.
Adds several custom HTML templates/expected-output fixtures for printing-template scenarios.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`tests/testprinting.h`	Declares the new `TestPrinting` test class and helpers.
`tests/testprinting.cpp`	Implements HTML export and render-based assertions for template substitution / profile placeholder recognition.
`tests/CMakeLists.txt`	Registers the new test target and sets Linux test environment for headless Qt execution.
`tests/printing_templates/custom_variant_self_closing.html`	Custom template fixture variant to exercise placeholder parsing.
`tests/printing_templates/custom_variant_extra_classes.html`	Custom template fixture variant to exercise placeholder parsing with additional classes.
`tests/printing_templates/custom_variant_style_attributes.html`	Custom template fixture variant to exercise placeholder parsing with style/attributes.
`printing_templates/tests/custom_template_1.html`	Additional printing template fixture (test data).
`printing_templates/tests/custom_template_1_output.html`	Expected output fixture for `custom_template_1.html`.
`printing_templates/tests/custom_template_2.html`	Additional printing template fixture (test data).
`printing_templates/tests/custom_template_2_output.html`	Expected output fixture for `custom_template_2.html`.
`printing_templates/tests/custom_template_3.html`	Additional printing template fixture (test data).
`printing_templates/tests/custom_template_3_output.html`	Expected output fixture for `custom_template_3.html`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-01T12:17:43Z

tests/testprinting.cpp

+	QVERIFY(!divelog.dives.empty());
+	const int diveId = divelog.dives[0]->id;
+
+	const QString prefix = templatePath.startsWith("/printing_templates/")


templatePath includes the SUBSURFACE_SOURCE prefix (an absolute path), so templatePath.startsWith("/printing_templates/") will never be true. If you meant to detect bundled templates, use something like templatePath.contains("/printing_templates/") or compare against QDir(SUBSURFACE_SOURCE "/printing_templates").absolutePath().

Suggested change

const QString prefix = templatePath.startsWith("/printing_templates/")

const QString prefix = templatePath.contains("/printing_templates/")

Copilot · 2026-03-01T12:17:43Z

printing_templates/tests/custom_template_2.html

+			background-color: {{ template_options.color3 }};
+			color: {{ template_options.color5 }};
+			width: 13%;
+			padding: o$ 1% 0% 1%;


This CSS line has a typo (padding: o$ ...) which results in invalid CSS in the template. It looks like it should be padding: 0 1% 0% 1%; (or similar).

Suggested change

padding: o$ 1% 0% 1%;

padding: 0 1% 0% 1%;

Copilot · 2026-03-01T12:17:43Z

printing_templates/tests/custom_template_2_output.html

+			background-color: #eff7ff;
+			color: #204a87;
+			width: 13%;
+			padding: o$ 1% 0% 1%;


This CSS line has a typo (padding: o$ ...) which results in invalid CSS in the generated expected output. It looks like it should be padding: 0 1% 0% 1%; (or similar).

Suggested change

padding: o$ 1% 0% 1%;

padding: 0 1% 0% 1%;

Copilot · 2026-03-01T12:17:43Z

printing_templates/tests/custom_template_2.html

+		body {
+			{{ print_options.grayscale }};
+			padding: 0;
+			margin: 0 0 0 6%; <!-- Provide LH margin for binding the page -->


HTML comments () aren’t valid inside CSS blocks and may be treated as invalid CSS by some renderers. Use a CSS comment (/* ... */) instead for this note.

Suggested change

margin: 0 0 0 6%; 

margin: 0 0 0 6%; /* Provide LH margin for binding the page */

Copilot · 2026-03-01T12:17:44Z

printing_templates/tests/custom_template_2_output.html

+		body {
+			;
+			padding: 0;
+			margin: 0 0 0 6%; <!-- Provide LH margin for binding the page -->


HTML comments () aren’t valid inside CSS blocks and may be treated as invalid CSS by some renderers. Use a CSS comment (/* ... */) instead for this note.

Suggested change

margin: 0 0 0 6%; 

margin: 0 0 0 6%; /* Provide LH margin for binding the page */

Copilot · 2026-03-01T12:17:44Z

tests/testprinting.cpp

+QString TestPrinting::createTemplateFromString(const QString &prefix, const QString &contents) const
+{
+	const QString templateName = QString("%1_%2.html").arg(prefix).arg(QDateTime::currentMSecsSinceEpoch());
+	QDir().mkpath(getPrintingTemplatePathUser());
+	QFile templateFile(getPrintingTemplatePathUser() + QDir::separator() + templateName);
+	if (!templateFile.open(QFile::WriteOnly | QFile::Text))
+		return QString();
+	QTextStream stream(&templateFile);
+	stream << contents;
+	templateFile.close();
+	return templateName;


createTemplateFromString() writes into the real user template directory returned by getPrintingTemplatePathUser() (typically ~/.subsurface/printing_templates). This makes the test suite mutate a developer/CI user’s home directory and can leave artifacts behind if the test aborts before cleanup() runs. Consider isolating the test by setting HOME (and any other relevant XDG vars) to a QTemporaryDir early in main() (before the first call to system_default_directory() / getPrintingTemplatePathUser()), or by otherwise redirecting the template path to a temp location for the duration of the test.

Copilot · 2026-03-01T12:17:44Z

tests/testprinting.cpp

+	const QString templateName = QString("%1_%2.html").arg(prefix).arg(QDateTime::currentMSecsSinceEpoch());
+	QDir().mkpath(getPrintingTemplatePathUser());


The template filename is based only on currentMSecsSinceEpoch(). Two templates created within the same millisecond (e.g., on a fast machine or in a tight loop) can collide and cause flaky failures/overwrites. Use a stronger uniqueness source (e.g., QUuid, QTemporaryFile, or append an incrementing counter) for the generated template name.

Copilot · 2026-03-01T12:17:44Z

tests/testprinting.cpp

+int TestPrinting::imageDifferencePixels(const QImage &a, const QImage &b) const
+{
+	if (a.size() != b.size())
+		return 0;


imageDifferencePixels() returns 0 when image sizes differ. That makes size-mismatch scenarios look like “no differences”, which can hide failures. Return a sentinel (e.g., -1) and fail the test when sizes differ, or treat size mismatch as a full difference count.

Suggested change

return 0;

return -1;

github-actions · 2026-03-01T12:20:07Z

Artifacts:
Subsurface-Linux-AppImage-6.0.5553-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-01T12:21:59Z

Artifacts:
Subsurface-MacOS-arm64-qt-6.8.3-6.0.5553-patch.1.pull-request.add-printing-tests
Subsurface-MacOS-x86_64-qt-5-6.0.5553-patch.1.pull-request.add-printing-tests
Subsurface-MacOS-x86_64-qt-6.8.3-6.0.5553-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-01T12:23:23Z

Artifacts:
Subsurface-Android-6.0.5553-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-01T12:28:05Z

Artifacts:
Subsurface-Windows-6.0.5553-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-01T12:49:54Z

Artifacts:
Subsurface-Windows-MSVC-qt-6-6.0.5553-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

dirkhh

I like the idea.
I'm curious about the Copilot comments (as usual). I wish I trusted Copilot more. My experience has been SO much better with Claude...

What I don't quite understand after reading the file twice is what we are actually testing here. Just whether we successfully parse the templates? Not some actual print output, right?

mikeller · 2026-03-20T23:09:19Z

What I don't quite understand after reading the file twice is what we are actually testing here. Just whether we successfully parse the templates? Not some actual print output, right?

We are testing two things - that processed html includes sections for dives, and that the dive profile is rendered (by comparing the rendered output to output from a modified template where profile rendering is intentionally disabled).

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-20T23:23:54Z

tests/CMakeLists.txt

+	add_executable(TestPrinting
+		testprinting.cpp
+		testprinting.h
+		../desktop-widgets/printer.cpp
+		../desktop-widgets/templatelayout.cpp
+	)
+	target_link_libraries(
+		TestPrinting
+		subsurface_backend_shared
+		${TEST_SPECIFIC_LIBRARIES}
+		subsurface_corelib
+		RESOURCE_LIBRARY
+		${QT_TEST_LIBRARIES}
+		${SUBSURFACE_LINK_LIBRARIES}
+	)
+	add_test(NAME TestPrinting COMMAND $<TARGET_FILE:TestPrinting>)
+	set(TEST_PRINTING TestPrinting)
+endif()



This test bypasses the existing TEST() helper used for the other test executables, which can lead to inconsistent ctest invocation (e.g., missing ${CFGARG} handling for multi-config generators) and inconsistent environment setup (the Linux QT_QPA_PLATFORM=offscreen property is applied inside the TEST() function, but not here). A tangible fix is to register TestPrinting via the same TEST() mechanism (and extend it / add an optional parameter for extra sources, or call set_tests_properties(TestPrinting ...) here to mirror the environment and CFGARG behavior).

Suggested change

add_executable(TestPrinting

testprinting.cpp

testprinting.h

../desktop-widgets/printer.cpp

../desktop-widgets/templatelayout.cpp

)

target_link_libraries(

TestPrinting

subsurface_backend_shared

${TEST_SPECIFIC_LIBRARIES}

subsurface_corelib

RESOURCE_LIBRARY

${QT_TEST_LIBRARIES}

${SUBSURFACE_LINK_LIBRARIES}

)

add_test(NAME TestPrinting COMMAND $<TARGET_FILE:TestPrinting>)

set(TEST_PRINTING TestPrinting)

endif()

TEST(TestPrinting

testprinting.cpp

../desktop-widgets/printer.cpp

../desktop-widgets/templatelayout.cpp

)

set(TEST_PRINTING TestPrinting)

endif()

Copilot · 2026-03-20T23:23:55Z

tests/testprinting.cpp

+	int differences = 0;
+	for (int y = 0; y < a.height(); ++y) {
+		for (int x = 0; x < a.width(); ++x) {
+			if (a.pixelColor(x, y) != b.pixelColor(x, y))
+				++differences;
+		}
+	}


pixelColor(x, y) is relatively expensive because it constructs QColor per pixel; this can make the test noticeably slower (images are 1200x800 and the comparison runs at least twice per data row). A faster approach is to compare scanlines using constScanLine()/scanLine() and compare QRgb (or bytes) directly, possibly early-exiting once you exceed your threshold (e.g., >200).

Suggested change

int differences = 0;

for (int y = 0; y < a.height(); ++y) {

for (int x = 0; x < a.width(); ++x) {

if (a.pixelColor(x, y) != b.pixelColor(x, y))

++differences;

}

}

int differences = 0;

const int width = a.width();

const int height = a.height();

for (int y = 0; y < height; ++y) {

const QRgb *lineA = reinterpret_cast<const QRgb *>(a.constScanLine(y));

const QRgb *lineB = reinterpret_cast<const QRgb *>(b.constScanLine(y));

for (int x = 0; x < width; ++x) {

if (lineA[x] != lineB[x])

++differences;

}

}

Copilot · 2026-03-20T23:23:55Z

tests/testprinting.cpp

+QString TestPrinting::exportHtmlWithTemplate(const QString &templateName) const
+{
+	print_options printOptions{};
+	printOptions.type = print_options::DIVELIST;
+	printOptions.p_template = templateName;
+	printOptions.print_selected = true;
+	printOptions.color_selected = true;
+	printOptions.landscape = false;
+	printOptions.resolution = 300;
+
+	template_options templateOptions{};
+	templateOptions.font_index = 0;
+	templateOptions.color_palette_index = SSRF_COLORS;
+	templateOptions.border_width = 1;
+	templateOptions.font_size = 9;
+	templateOptions.line_spacing = 1;
+	templateOptions.color_palette.color1 = QColor::fromRgb(0xff, 0xff, 0xff);
+	templateOptions.color_palette.color2 = QColor::fromRgb(0xa6, 0xbc, 0xd7);
+	templateOptions.color_palette.color3 = QColor::fromRgb(0xef, 0xf7, 0xff);
+	templateOptions.color_palette.color4 = QColor::fromRgb(0x34, 0x65, 0xa4);
+	templateOptions.color_palette.color5 = QColor::fromRgb(0x20, 0x4a, 0x87);
+	templateOptions.color_palette.color6 = QColor::fromRgb(0x17, 0x37, 0x64);


print_options and template_options initialization is duplicated in both exportHtmlWithTemplate() and renderWithTemplate(). This increases maintenance cost if defaults change (e.g., palette, resolution). Consider extracting a small helper (e.g., a function that returns a populated {print_options, template_options} pair given templateName) and reuse it in both codepaths.

github-actions · 2026-03-20T23:29:14Z

Artifacts:
Subsurface-Linux-AppImage-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-20T23:32:02Z

Artifacts:
Subsurface-Android-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-20T23:32:25Z

Artifacts:
Subsurface-MacOS-x86_64-qt-6.8.3-6.0.5574-patch.1.pull-request.add-printing-tests
Subsurface-MacOS-x86_64-qt-5-6.0.5574-patch.1.pull-request.add-printing-tests
Subsurface-MacOS-arm64-qt-6.8.3-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-20T23:34:54Z

Artifacts:
Subsurface-Windows-MSVC-qt-6-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-20T23:36:48Z

Artifacts:
Subsurface-Windows-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

Add tests to verify that the profile substitution works for the default templates and a number of custom templates with different html formatting. Signed-off-by: Michael Keller <[email protected]> Add Test Files for Printing Tests. Add custom templates to test that html input is properly supported for printing. Signed-off-by: Michael Keller <[email protected]>

github-actions · 2026-03-21T01:50:06Z

Artifacts:
Subsurface-Linux-AppImage-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-21T01:53:10Z

Artifacts:
Subsurface-Windows-MSVC-qt-6-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-21T01:56:36Z

Artifacts:
Subsurface-Windows-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

github-actions · 2026-03-21T01:56:43Z

Artifacts:
Subsurface-MacOS-x86_64-qt-6.8.3-6.0.5574-patch.1.pull-request.add-printing-tests
Subsurface-MacOS-x86_64-qt-5-6.0.5574-patch.1.pull-request.add-printing-tests
Subsurface-MacOS-arm64-qt-6.8.3-6.0.5574-patch.1.pull-request.add-printing-tests
WARNING: Use at your own risk.

Copilot AI review requested due to automatic review settings March 1, 2026 12:11

Copilot started reviewing on behalf of mikeller March 1, 2026 12:11 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

dirkhh reviewed Mar 1, 2026

View reviewed changes

mikeller force-pushed the add_printing_tests branch from 2222953 to 2bc85a2 Compare March 20, 2026 23:20

mikeller requested a review from Copilot March 20, 2026 23:21

Copilot AI reviewed Mar 20, 2026

View reviewed changes

mikeller force-pushed the add_printing_tests branch from 2bc85a2 to 1c9e226 Compare March 21, 2026 01:38

mikeller merged commit ffca89e into subsurface:master Mar 21, 2026
30 checks passed

mikeller deleted the add_printing_tests branch March 21, 2026 02:26

	const QString prefix = templatePath.startsWith("/printing_templates/")
	const QString prefix = templatePath.contains("/printing_templates/")

	margin: 0 0 0 6%; <!-- Provide LH margin for binding the page -->
	margin: 0 0 0 6%; /* Provide LH margin for binding the page */

		const QString templateName = QString("%1_%2.html").arg(prefix).arg(QDateTime::currentMSecsSinceEpoch());
		QDir().mkpath(getPrintingTemplatePathUser());

Uh oh!

Conversation

mikeller commented Mar 1, 2026

Describe the pull request:

Pull request long description:

Changes made:

Related issues:

Additional information:

Documentation change:

Mentions:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 1, 2026

Uh oh!

github-actions bot commented Mar 1, 2026

Uh oh!

github-actions bot commented Mar 1, 2026

Uh oh!

github-actions bot commented Mar 1, 2026

Uh oh!

github-actions bot commented Mar 1, 2026

Uh oh!

dirkhh left a comment

Choose a reason for hiding this comment

Uh oh!

mikeller commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 21, 2026

Uh oh!