Skip to content

CONFORMITY_TESTING.md#324

Merged
nlothian merged 1 commit into
mainfrom
fix/compliance_report
Dec 6, 2025
Merged

CONFORMITY_TESTING.md#324
nlothian merged 1 commit into
mainfrom
fix/compliance_report

Conversation

@nlothian

@nlothian nlothian commented Dec 6, 2025

Copy link
Copy Markdown
Owner

No description provided.

@nlothian
nlothian merged commit 818b208 into main Dec 6, 2025
2 checks passed
@nlothian
nlothian deleted the fix/compliance_report branch December 6, 2025 07:34

@kilo-code-bot kilo-code-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No Issues Found

3 files reviewed | Confidence: 95% | Recommendation: Merge (already merged)

Files:

  • docs/CONFORMITY_TESTING.md (documentation update)
  • tools/conformity_test.py (framework enhancement)
  • tools/scrape_conformity_expected.py (new utility script)

Checked: Security, bugs, performance, error handling, code clarity

Summary: This PR significantly improves the ISO Prolog conformity testing framework by expanding from 10 to 355 tests, adding expected result validation, and including reference implementation comparisons. The changes enhance test accuracy and reporting quality without introducing security risks or bugs.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly expands the ISO Prolog conformity test suite from 10 to 355 tests, introducing a more robust framework for evaluating compliance. The changes correctly distinguish between failing tests and tests that are expected to produce errors, using the term 'conforming' for more accurate reporting. The inclusion of reference results from SWI-Prolog and Scryer-Prolog is also a valuable addition for comparison. The overall changes are excellent and greatly improve the project's testing infrastructure. I've identified a couple of minor areas for improvement: some comments in the test expectations are mismatched with the test cases, which should be clarified to align with documentation principles for non-standard features, and a new utility script includes an unused import. These are minor points in an otherwise high-quality contribution.

Comment thread tools/conformity_test.py
Comment on lines +60 to +65
21: TestResult.SYNTAX_ERROR, # \e in char_code
22: TestResult.SYNTAX_ERROR, # \d in char_code
23: TestResult.SYNTAX_ERROR, # \u1 incomplete
24: TestResult.SYNTAX_ERROR, # \u0021 in atom
25: TestResult.SYNTAX_ERROR, # \u0021 in char code
26: TestResult.SYNTAX_ERROR, # \u0021 in string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The comments for tests 21-26 in _EXPECTED_RESULTS do not seem to match the actual test queries, which could be confusing for future maintenance. For example, test 21 is writeq('\\e'). but the comment is # \\e in char_code. Please review and correct these comments for clarity. This aligns with the principle of explicitly documenting deviations from standard behavior, even if not directly related to predicates.

Suggested change
21: TestResult.SYNTAX_ERROR, # \e in char_code
22: TestResult.SYNTAX_ERROR, # \d in char_code
23: TestResult.SYNTAX_ERROR, # \u1 incomplete
24: TestResult.SYNTAX_ERROR, # \u0021 in atom
25: TestResult.SYNTAX_ERROR, # \u0021 in char code
26: TestResult.SYNTAX_ERROR, # \u0021 in string
21: TestResult.SYNTAX_ERROR, # `\\e` is not a standard escape sequence
22: TestResult.SYNTAX_ERROR, # Incomplete octal escape `\\033\\`
23: TestResult.SYNTAX_ERROR, # Incomplete octal escape `\\0\\`
24: TestResult.SYNTAX_ERROR, # `\\e` is not a standard character for char_code/2
25: TestResult.SYNTAX_ERROR, # `\\d` is not a standard character for char_code/2
26: TestResult.SYNTAX_ERROR, # Incomplete unicode escape `\\u1`
References
  1. Non-standard extensions to ISO-compliant predicates are permissible, but they must be explicitly documented as extensions. This principle extends to documenting non-standard escape sequences/characters for clarity.

python tools/scrape_conformity_expected.py
"""

import sys

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The sys module is imported but never used in this script. It's good practice to remove unused imports to keep the code clean and avoid potential confusion.

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.

1 participant