Skip to content

Make GoldenFileTest self-contained in CI#1

Merged
chrjohn merged 2 commits into
masterfrom
copilot/fix-golden-file-test-dependency
Jun 3, 2026
Merged

Make GoldenFileTest self-contained in CI#1
chrjohn merged 2 commits into
masterfrom
copilot/fix-golden-file-test-dependency

Conversation

Copilot AI commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

GoldenFileTest depended on FIX42/FIX44 dictionaries from a sibling quickfixj-messages checkout, which does not exist in CI. This updates the test to use in-repo fixtures so the Maven workflow can run from a single repository checkout.

  • Test inputs

    • Added vendored FIX dictionaries under src/test/resources/dictionaries/:
      • FIX42.xml
      • FIX44.xml
  • GoldenFileTest

    • Repointed the test to the in-repo dictionary files instead of ../quickfixj-messages/...
    • Kept the change scoped to the existing golden-file regression path
  • Documentation

    • Updated the golden test README to reference the new dictionary locations
private File fix42DictFile = new File("./src/test/resources/dictionaries/FIX42.xml");
private File fix44DictFile = new File("./src/test/resources/dictionaries/FIX44.xml");
Original prompt

Create the smallest, most robust fix for the failing CI job in repository quickfix-j/quickfixj-codegenerator.

Context:

  • The failing job is from workflow .github/workflows/maven.yml in run 26900115940, job 79349879432.
  • The failure is in src/test/java/org/quickfixj/codegenerator/GoldenFileTest.java.
  • The test currently depends on FIX dictionary XML files located outside this repository via relative paths:
    • ../quickfixj-messages/quickfixj-messages-fix42/src/main/resources/FIX42.xml
    • ../quickfixj-messages/quickfixj-messages-fix44/src/main/resources/FIX44.xml
  • CI only checks out this repository, so those files do not exist, causing FileNotFoundException during test execution.

Requested solution:

  • Implement the smallest, most robust solution.
  • Make the test self-contained by storing the required FIX XML dictionaries inside this repository under test resources.
  • Update GoldenFileTest to read the dictionary files from this repository instead of from a sibling checkout.

Expected changes:

  1. Add these files to the repo:
    • src/test/resources/dictionaries/FIX42.xml
    • src/test/resources/dictionaries/FIX44.xml
  2. Update src/test/java/org/quickfixj/codegenerator/GoldenFileTest.java so the dictionary file paths point to those new test resource files.
  3. Keep the change minimal and avoid unrelated refactors.
  4. Ensure the resulting tests pass in CI without requiring any additional repository checkout.

Please open a pull request with the fix.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The failure is caused by GoldenFileTest depending on FIX XML dictionaries that are not present in this repository checkout.

Root cause

The test hard-codes dictionary paths outside the repo:

It expects:

  • ../quickfixj-messages/quickfixj-messages-fix42/src/main/resources/FIX42.xml
  • ../quickfixj-messages/quickfixj-messages-fix44/src/main/resources/FIX44.xml

But the workflow only checks out this repo and runs Maven:

So on CI, those files do not exist, producing the FileNotFoundException seen in the failing job.

Best fix

Make the test self-contained by using dictionaries stored in this repository’s test resources instead of referencing a sibling checkout.

Suggested code change

Update GoldenFileTest to load FIX42/FIX44 XML from src/test/resources:

private File fix42DictFile = new File("./src/test/resources/dictionaries/FIX42.xml");
private File fix44DictFile = new File("./src/test/resources/dictionaries/FIX44.xml");

Then add these files to the repo:

  • src/test/resources/dictionaries/FIX42.xml
  • src/test/resources/dictionaries/FIX44.xml

This is the most reliable solution because it removes the CI/environment dependency entirely.

Recommended defensive improvement

Fail fast with a clear assertion before generation instead of allowing a deep FileNotFoundException from parsing:

@Before
public void setup() {
    generator = new MessageCodeGenerator();
    assertDictionaryExists(fix42DictFile);
    assertDictionaryExists(fix44DictFile);
}

private void assertDictionaryExists(File file) {
    if (!file.isFile()) {
        fail("Missing FIX dictionary required for test: " + file.getPath());
    }
}

Why this is the right fix

MessageCodeGenerator parses the specification file directly via builder.parse(task.getSpecification()):

So the failure is not in generation logic; it is purely test setup and repository layout.

Alternative if you do not want to vendor the XML files

Change the workflow to also check out the quickfixj-messages repository into ../quickfixj-messages, but this is less desirable because it makes tests dependent on multi-repo CI layout. The cleaner solution is to keep test inputs in this repo.

Minimal patch shape

private File fix42DictFile =
        new File("./src/test/resources/dictionaries/FIX42.xml");
private File fix44DictFile =
        new File("./src/test/resources/dictionaries/FIX44.xml");

If you wan...

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Fix GoldenFileTest dependency on external FIX XML files Make GoldenFileTest self-contained in CI Jun 3, 2026
Copilot AI requested a review from chrjohn June 3, 2026 17:08
@chrjohn chrjohn added this to the 3.0.2 milestone Jun 3, 2026
@chrjohn chrjohn marked this pull request as ready for review June 3, 2026 17:18
@chrjohn chrjohn merged commit 243cdcf into master Jun 3, 2026
12 checks passed
@chrjohn chrjohn deleted the copilot/fix-golden-file-test-dependency branch June 3, 2026 17:20
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.

2 participants