Skip to content

Add ICD coverage for the AIPS beam support flag in OPEN_FILE - #88

Draft
markccchiang wants to merge 6 commits into
devfrom
mark/update_open_file_tests
Draft

Add ICD coverage for the AIPS beam support flag in OPEN_FILE#88
markccchiang wants to merge 6 commits into
devfrom
mark/update_open_file_tests

Conversation

@markccchiang

Copy link
Copy Markdown
Collaborator

Description

Closes #33. Adds ICD coverage for OpenFile.support_aips_beam. The flag was not merely untested; it was unsendable: MessageController hardcoded false in both loadFile() and getFileInfo(), so no test could have exercised it. This plumbs it through, adds a fixture generator, and adds the test.

Changes

File
src/test/MessageController.ts plumb supportAipsBeam through loadFile() and getFileInfo(); both default to false, so the other 126 test files are unaffected
src/test/OPENFILE_AIPS_BEAM.test.ts new — 15 tests over OPEN_FILE and FILE_INFO_REQUEST
scripts/make_aips_history_beam_fixture.py new — generates the fixture images deterministically
ICD_test_stages/file_browser.tests register the test in the file-browser stage
docs/source/open_file.rst document the four checks

Fixture

No AIPS-history-beam image existed in the test data set, so one is generated: a 128×128 FITS with no BMAJ/BMIN/BPA keywords and no BEAMS table, carrying the beam only in HISTORY cards.

HISTORY AIPS   CLEAN BMAJ=  1.111111E-03 BMIN=  8.333333E-04 BPA=  10.00
HISTORY AIPS   CLEAN BMAJ=  5.555556E-04 BMIN=  4.166667E-04 BPA=  30.00

Two cards, decoy first, so the test pins that the backend keeps the last beam. The beam is deliberately non-circular (2.0" × 1.5") so an axis mix-up is detectable; both choices caught real bugs. The card format is not free: casacore's ImageInfo::getRestoringBeam requires whitespace-separated BMAJ/BMIN/BPA tokens, and without a casacore-recognised beam the backend feature never fires at all.

Two new sample image files aips_history_beam.fits (set_QA) and aips_history_beam.fits.gz
(set_compressed_fits) were uploaded to the shared test dataset; the file-browser stage reddens on all 7 CI runners.

Backend findings

Three bugs surfaced. None are encoded as expected behaviour; assertions are written to pass before and after a fix, with strict versions skipped alongside.

  1. BMAJ/BMIN header entries are swapped: FileExtInfoLoader.cc:1243 assigns the major axis to BMIN and vice versa. The computed Restoring beam string and the log line are both correct. The live test compares the axes as an unordered pair; a skipped test pins the intended assignment.

  2. Compressed FITS never reports the AIPS beam: the _support_aips_beam branch in FitsLoader::ResetImageBeam is gated on casacore having already found a beam, which CartaFitsImage (used for in-memory .gz) never does. Covered by a describe.skip.

  3. FILE_INFO_REQUEST returns stale info when the flag flips: ResetImageBeam only runs at image-allocation time, and OnFileInfoRequest reuses a cached loader whose image was already stripped. OPEN_FILE escapes because it evicts the loader.

(2) and (3) are two failure modes of the same design. The beam is resolved once, at allocation, behind a gate, and is probably one backend issue.

Checklist

For the pull request:

  • Documentation has been updated (or no documentation changes are needed)

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.

Add test for FileOpen new sub-message

1 participant