Test framework by wrp · Pull Request #116 · heasarc/cfitsio

wrp · 2026-01-15T17:44:12Z

Add a simplistic test framework. testprog only exercises about 16% of the code (as reported by gcov), and 22 files have zero coverage. This adds some basic unit testing to exercise a larger chunk of the code base to protect against regressions.

Note that, other than trivial changes to Makefile.am and configure.ac (and the subsequent changes to the autogenerated Makefile.in and configure) and a change to comments in fits_hdcompress.c, this PR is just adding files in tests/. Although the PR is large, it does not modify the existing code in any significant way.

esabol · 2026-01-15T21:49:06Z

This is a huge contribution. Unit tests have long been much needed. Thank you!

The CI is currently failing with:

ERROR: files left in build directory after distclean:
./test_putcoluj2.fits

esabol · 2026-01-15T21:51:09Z

A few issues discovered while developing the tests are mentioned in tests/ISSUES

I recommend opening separate issues for these in https://github.com/HEASARC/cfitsio/issues/

I would then delete the ISSUES file from the pull request.

wrp · 2026-01-16T05:52:05Z

Removed ISSUES and created issues 117-119. PTAL

wrp · 2026-01-16T15:23:45Z

Checking the test failures on ubuntu.

esabol · 2026-01-20T19:26:12Z

All CI workflows are passing now. LGTM.

wrp · 2026-01-27T14:43:03Z

Rebased on d5d4888

wrp · 2026-02-05T12:42:56Z

Rebased on 28f9c12, addressed juice928's comment regarding /tmp
files in tests, and added tests to exercise the fixes in b767730,
8441ed1, and 9400723.

esabol

Fantastic! Thanks, @wrp !

The pseudo-random number generator in test_rcomp_high_entropy() used signed integer arithmetic that overflows when i >= 2: original[i] = ((i * 1103515245 + 12345) >> 16) & 0x7FFF; The multiplication i * 1103515245 exceeds INT_MAX (2147483647) at i=2, producing 2207030490 which cannot be represented as a signed int. This is undefined behavior per the C standard (C11 6.5/5). GCC on Ubuntu ARM with -O2 exploits this UB via aggressive loop optimizations (-Waggressive-loop-optimizations), causing the test to abort with a core dump during distcheck. The compiler warning: warning: iteration 2 invokes undefined behavior The fix uses unsigned arithmetic throughout: original[i] = (((unsigned)i * 1103515245U + 12345U) >> 16) & 0x7FFF; This produces identical pseudo-random values but with well-defined overflow semantics (modulo 2^32 per C11 6.2.5/9).

The fits_rcomp, fits_rcomp_short, and fits_rcomp_byte functions do not perform end-of-buffer checking. As documented in ricecomp.c: "Note that beginning with CFITSIO v3.08, EOB checking was removed to improve speed, and so now the input compressed bytes buffers must have been allocated big enough so that they will never be overflowed." The removed tests passed intentionally undersized buffers expecting the library to detect this and return an error. Instead, the library writes past the buffer bounds, causing stack corruption. This was detected on Linux by FORTIFY_SOURCE/glibc, triggering SIGABRT. The test_rdecomp_buffer_too_small test is retained since decompression reads from the buffer (bounded by clen parameter) rather than writing.

Test integer division overflow (LONG_MIN / -1), division by zero with column data, vector index out-of-range, and double column arithmetic to exercise the guards added in b767730, 8441ed1, and 9400723.

fits_hdecompress was modified to take an additional argument, and this commit adds that new argument to the tests.

krutkow · 2026-05-28T01:43:19Z

 #AC_CONFIG_HEADERS(config.h)
 AC_CONFIG_SRCDIR([fitscore.c])
 AM_INIT_AUTOMAKE([foreign])
+AM_SILENT_RULES([yes])


This edits the output of the make system and is unrelated to testing. Please remove. (and update the generated configure and Makefile.in files)

krutkow · 2026-05-28T02:01:45Z


 # Distribution: ==========================================================

+DISTCHECK_CONFIGURE_FLAGS = --enable-reentrant


This is unrelated to the test framework and conflicts/is redundant with our existing CI, which already tests reentrant. Please remove.

krutkow · 2026-05-28T16:34:58Z

now that PR133 is merged, you can merge develop into this PR and remove your fits_hdecompress.c commit from this PR.

cruzzil · 2026-06-21T13:45:59Z

+ * Test ffcalc with LONGLONG column
+ */
+static void
+test_ffcalc_longlong_column(void)


This test should fail on any platform where long is 32bits instead of 64 bits.

cruzzil · 2026-06-21T13:47:28Z

+
+#define test_path "test_grparser.fits"
+#define template_path "test_grparser.tpl"
+


To run on windows you'll need:

Suggested change

#ifdef _WIN32

/* MSVC lacks POSIX setenv/unsetenv; emulate with _putenv. */

static int

setenv(const char *name, const char *value, int overwrite)

{

char buf[1024];

(void) overwrite;

snprintf(buf, sizeof(buf), "%s=%s", name, value);

return _putenv(buf);

}

static int

unsetenv(const char *name)

{

char buf[1024];

snprintf(buf, sizeof(buf), "%s=", name);

return _putenv(buf);

}

#endif

cruzzil · 2026-06-21T13:48:23Z

+	int status = 0;
+	char *ttype[] = { "DBLCOL" };
+	char *tform[] = { "1D" };
+	double nan_val = 0.0 / 0.0;  /* Generate NaN. */


For windows:

Suggested change

double nan_val = 0.0 / 0.0; /* Generate NaN. */

/* 'zero' is volatile so the division is performed at runtime rather

than folded by the compiler. A literal 0.0/0.0 is a compile-time

constant expression that MSVC rejects with error C2124 ("divide or

mod by zero"); the volatile forces a genuine runtime NaN. */

volatile double zero = 0.0;

double nan_val = zero / zero; /* Generate NaN. */

cruzzil · 2026-06-21T13:48:48Z

+	int status = 0;
+	char *ttype[] = { "DBLCOL" };
+	char *tform[] = { "1D" };
+	double inf_val = 1.0 / 0.0;  /* Generate infinity. */


For windows:

Suggested change

double inf_val = 1.0 / 0.0; /* Generate infinity. */

/* volatile for the same reason as in test_write_nan_values(): a literal

1.0/0.0 is a compile-time constant that MSVC rejects (C2124), so the

operands are made volatile to force a runtime division to infinity. */

volatile double one = 1.0, zero = 0.0;

double inf_val = one / zero; /* Generate infinity. */

cruzzil · 2026-06-21T13:50:20Z

+	int status = 0;
+	char *ttype[] = { "FLOATCOL" };
+	char *tform[] = { "1E" };
+	float nan_val = 0.0f / 0.0f;  /* Generate NaN. */


For windows:

Suggested change

float nan_val = 0.0f / 0.0f; /* Generate NaN. */

/* 'zero' is volatile so the division is performed at runtime rather

than folded by the compiler. A literal 0.0f/0.0f is a compile-time

constant expression that MSVC rejects with error C2124 ("divide or

mod by zero"); the volatile forces a genuine runtime NaN. */

volatile float zero = 0.0f;

float nan_val = zero / zero; /* Generate NaN. */

AI port of heasarc/cfitsio#116 test framework. Huge increase in code coverage and detected many bugs

wrp force-pushed the test-framework branch 2 times, most recently from 15514cb to 1866a9e Compare January 15, 2026 21:34

wrp force-pushed the test-framework branch from 1866a9e to 9d3c159 Compare January 16, 2026 05:51

wrp force-pushed the test-framework branch 2 times, most recently from c73266f to 3b66baa Compare January 18, 2026 01:07

esabol approved these changes Jan 20, 2026

View reviewed changes

wrp force-pushed the test-framework branch from 3b66baa to 9fe957c Compare January 27, 2026 14:42

wrp force-pushed the test-framework branch from 9fe957c to 2ce5e0d Compare February 5, 2026 12:42

esabol approved these changes Feb 5, 2026

View reviewed changes

wrp force-pushed the test-framework branch from 2ce5e0d to 9ed4078 Compare February 10, 2026 00:54

wrp added 13 commits April 11, 2026 06:25

Add test_macros.h with test helper macros

6c1064e

Add tests.am automake include file

ae3c1a4

Add tests for buffers.c

2bc7967

Add tests for cfileio.c

379f969

Add tests for checksum.c

5dd6c73

Add tests for drvrsmem.c

525f2c2

Add tests for editcol.c

ac73e60

Add tests for edithdu.c

9fcaf66

Add tests for eval.c

8363fcb

Add tests for fileops.c

31d5d50

Add tests for find_match_delim.c

eb9d50e

Add tests for getcol.c

77b4bfb

Add tests for getcolb.c

c2af739

wrp added 20 commits April 11, 2026 06:25

Add tests for putcolui.c

98ce0a4

Add tests for putcoluj.c

ead246e

Add tests for putcoluk.c

b0e5355

Add tests for putkey.c

d7e0cd1

Add tests for quantize.c

7ab6b8b

Add tests for region.c

4fa227f

Add tests for ricecomp.c

fdd2dc0

Add tests for scalnull.c

cd2fbf1

Add tests for simplerng.c

49cd6cc

Add tests for wcssub.c

937a63a

Add tests for wcsutil.c

ed28bb9

Update configure.ac for test framework

e655980

Update Makefile.am for test framework

30169df

Regenerate autoconf files

b1f4b5d

Add coverage-test to verify gcov thresholds

9a898d9

Add tests for recent eval.y fixes

72707e9

Test integer division overflow (LONG_MIN / -1), division by zero with column data, vector index out-of-range, and double column arithmetic to exercise the guards added in b767730, 8441ed1, and 9400723.

Describe the "na" parameter of fits_hdecompress

b919a1b

Adjust number of arguments to fits_hdecompress

c0b6b92

fits_hdecompress was modified to take an additional argument, and this commit adds that new argument to the tests.

wrp force-pushed the test-framework branch from 9ed4078 to c0b6b92 Compare April 11, 2026 12:51

krutkow requested changes May 28, 2026

View reviewed changes

cruzzil reviewed Jun 21, 2026

View reviewed changes

cruzzil mentioned this pull request Jun 26, 2026

Test framework cruzzil/rsfitsio#78

Merged

cruzzil added a commit to cruzzil/rsfitsio that referenced this pull request Jun 26, 2026

Merge pull request #78 from cruzzil/test_framework_expanded

c970496

AI port of heasarc/cfitsio#116 test framework. Huge increase in code coverage and detected many bugs


		# Distribution: ==========================================================

		DISTCHECK_CONFIGURE_FLAGS = --enable-reentrant


		#define test_path "test_grparser.fits"
		#define template_path "test_grparser.tpl"

+#ifdef _WIN32
+/* MSVC lacks POSIX setenv/unsetenv; emulate with _putenv. */
+static int
+setenv(const char *name, const char *value, int overwrite)
+{
+	char buf[1024];
+	(void) overwrite;
+	snprintf(buf, sizeof(buf), "%s=%s", name, value);
+	return _putenv(buf);
+}
+static int
+unsetenv(const char *name)
+{
+	char buf[1024];
+	snprintf(buf, sizeof(buf), "%s=", name);
+	return _putenv(buf);
+}
+#endif

-	double nan_val = 0.0 / 0.0;  /* Generate NaN. */
+	/* 'zero' is volatile so the division is performed at runtime rather
+	   than folded by the compiler.  A literal 0.0/0.0 is a compile-time
+	   constant expression that MSVC rejects with error C2124 ("divide or
+	   mod by zero"); the volatile forces a genuine runtime NaN. */
+	volatile double zero = 0.0;
+	double nan_val = zero / zero;  /* Generate NaN. */

-	double inf_val = 1.0 / 0.0;  /* Generate infinity. */
+	/* volatile for the same reason as in test_write_nan_values(): a literal
+.0/0.0 is a compile-time constant that MSVC rejects (C2124), so the
+	   operands are made volatile to force a runtime division to infinity. */
+	volatile double one = 1.0, zero = 0.0;
+	double inf_val = one / zero;  /* Generate infinity. */

Uh oh!

Conversation

wrp commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esabol commented Jan 15, 2026

Uh oh!

esabol commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wrp commented Jan 16, 2026

Uh oh!

wrp commented Jan 16, 2026

Uh oh!

esabol commented Jan 20, 2026

Uh oh!

wrp commented Jan 27, 2026

Uh oh!

wrp commented Feb 5, 2026

Uh oh!

esabol left a comment

Choose a reason for hiding this comment

Uh oh!

krutkow May 28, 2026

Choose a reason for hiding this comment

Uh oh!

krutkow May 28, 2026

Choose a reason for hiding this comment

Uh oh!

krutkow commented May 28, 2026

Uh oh!

cruzzil Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

cruzzil Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

cruzzil Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

cruzzil Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

cruzzil Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wrp commented Jan 15, 2026 •

edited

Loading

esabol commented Jan 15, 2026 •

edited

Loading