Skip to content

fix: improve ISA regex validation and add comprehensive test suite #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added .coverage
Binary file not shown.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [3.20.1] - 2025-07-04
- Fix ISA regex pattern to properly handle sub-extensions like 'RV32I_Zicsr' and add comprehensive test suite

## [3.20.0] - 2024-07-08
- Add Sdtrig support

Expand Down
30 changes: 30 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Makefile for riscv-config project

.PHONY: help test test-coverage clean install-test

help: ## Show this help message
@echo "Available targets:"
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-15s\033[0m %s\n", $$1, $$2}'

install-test: ## Install test dependencies
pip install pytest pytest-cov

test: ## Run all tests
pytest tests/test_constants.py -v

test-coverage: ## Run tests with coverage report
python -W ignore::SyntaxWarning -m pytest tests/test_constants.py --cov=riscv_config --cov-report=html
@echo "Coverage report generated in htmlcov/index.html"

test-coverage-constants: ## Run tests with coverage for constants module only
pytest tests/test_constants.py --cov=riscv_config.constants --cov-report=html --cov-report=term-missing
@echo "Focused coverage report for constants module"

clean: ## Clean up generated files
rm -rf htmlcov/
rm -rf .pytest_cache/
rm -rf __pycache__/
rm -rf tests/__pycache__/
rm -rf riscv_config/__pycache__/
find . -name "*.pyc" -delete
find . -name ".coverage" -delete
63 changes: 63 additions & 0 deletions TESTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Testing

Tests for the RISC-V ISA regex pattern.

## Quick Start

```bash
make help # See all commands
make install-test # Install dependencies
make test # Run tests
```

## What's Tested

- Valid ISA strings: `RV32I`, `RV64IG`, `RV32I_Zicsr`
- Invalid patterns: `RV64G`, `rv32i`, `RV32IZicsr`
- Edge cases and real configs

## Running Tests

```bash
make test # Basic test run
make test-coverage # Full coverage report
make test-coverage-constants # Coverage for regex only
```

## Adding Tests

Edit `tests/test_constants.py`:

```python
# Valid patterns
("RV32I_NewExt", True),

# Invalid patterns
("RV32I_BadExt", False),
```

## CI Setup

```yaml
# .github/workflows/test.yml
name: Tests
on: [push, pull_request]
jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
- run: make install-test
- run: make test
```

## Troubleshooting

**Import errors?** Make sure you're in project root:
```bash
cd /workspaces/riscv-config
export PYTHONPATH=.
```

**Coverage report?** Open `htmlcov/index.html` after running coverage tests.
4 changes: 4 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[tool:pytest]
testpaths = tests
python_files = test_*.py
addopts = -v --tb=short --cov=riscv_config
2 changes: 2 additions & 0 deletions requirements-test.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pytest>=7.0.0
pytest-cov>=4.0.0
2 changes: 1 addition & 1 deletion riscv_config/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from pkgutil import extend_path
__path__ = extend_path(__path__, __name__)
__version__ = '3.20.0'
__version__ = '3.20.1'
2 changes: 1 addition & 1 deletion riscv_config/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@
sub_extensions = Z_extensions + S_extensions

isa_regex = \
re.compile("^RV(32|64|128)[IE][ACDFGHJLMNPQSTUV]*(("+'|'.join(sub_extensions)+")(_("+'|'.join(sub_extensions)+"))*){,1}(X[a-z0-9]*)*(_X[a-z0-9]*)*$")
re.compile("^RV(32|64|128)[IE][ACDFGHJLMNPQSTUV]*(_("+'|'.join(sub_extensions)+")+)*(_X[a-zA-Z0-9]+)*$")
20 changes: 20 additions & 0 deletions run_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env python3
"""Simple test runner for ISA regex tests."""

import sys
import os

# Add project root to path
sys.path.insert(0, os.path.dirname(__file__))

if __name__ == "__main__":
try:
import pytest
sys.exit(pytest.main([
"tests/test_constants.py",
"-v",
"--tb=short"
]))
except ImportError:
print("pytest not installed. Install with: pip install pytest")
sys.exit(1)
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 3.20.0
current_version = 3.20.1
commit = True
tag = True

Expand Down
1 change: 1 addition & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Empty file to make tests a package
165 changes: 165 additions & 0 deletions tests/test_constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
"""Tests for ISA regex pattern validation."""

import pytest
import re
from riscv_config.constants import isa_regex


class TestISARegex:

@pytest.mark.parametrize("isa_string,expected", [
# Basic ISA strings
("RV32I", True),
("RV64I", True),
("RV128I", True),
("RV32E", True),

# Standard extensions
("RV32IMAFD", True),
("RV64IG", True),
("RV32IMC", True),
("RV64IMAFDC", True),

# Sub-extensions
("RV32I_Zicsr", True),
("RV64I_Zifencei", True),
("RV32I_Zicsr_Zifencei", True),
("RV32I_Svnapot", True),
("RV64I_Smrnmi", True),

# Custom extensions
("RV32I_Xvendor", True),
("RV64I_Xvendor1_Xvendor2", True),
("RV64I_XcustomExt123", True),

# Complex combinations
("RV64IMAFD_Zicsr_Zifencei_Xvendor", True),
("RV32I_Zve32x", True),
("RV64I_Zve64f", True),
("RV32I_Zvl32b", True),
])
def test_valid_isa_strings(self, isa_string, expected):
"""Valid ISA strings should match the regex."""
assert bool(isa_regex.match(isa_string)) == expected

@pytest.mark.parametrize("isa_string,expected", [
# Missing base ISA
("RV64G", False),
("RV32", False),
("RV64", False),

# Invalid widths
("RV16I", False),
("RV256I", False),

# Invalid base ISA
("RV32X", False),
("RV32II", False),
("RV64EI", False),

# Format errors
("RV32IZicsr", False),
("RV32I_", False),
("RV32I__Zicsr", False),
("RV32I_Zicsr_", False),

# Case sensitivity
("rv32i", False),
("RV32i", False),
("RV32I_zicsr", False),
("RV32I_ZICSR", False),

# Unknown extensions
("RV32I_Zunknown", False),
("RV32I_Sunknown", False),
("RV32I_X", False),
("RV32I_Xinvalid-name", False),

# Whitespace and special chars
("", False),
("RV32I ", False),
(" RV32I", False),
("RV32I@", False),
("RV32I_Zicsr!", False),
])
def test_invalid_isa_strings(self, isa_string, expected):
"""Invalid ISA strings should be rejected."""
assert bool(isa_regex.match(isa_string)) == expected

def test_regex_structure(self):
"""Basic regex structure tests."""
assert isinstance(isa_regex, re.Pattern)
assert isa_regex.pattern.startswith("^")
assert isa_regex.pattern.endswith("$")

def test_all_standard_extensions(self):
"""Test all standard extensions work."""
for ext in "ACDFGHJLMNPQSTUV":
isa = f"RV32I{ext}"
assert isa_regex.match(isa), f"Extension {ext} should work"

def test_all_architectures(self):
"""Test all supported architectures."""
for width in ["32", "64", "128"]:
for base in ["I", "E"]:
isa = f"RV{width}{base}"
assert isa_regex.match(isa), f"{isa} should match"

def test_extension_categories(self):
"""Test different extension categories."""
# Z extensions
z_tests = ["RV32I_Zicsr", "RV32I_Zba", "RV32I_Zfh"]
for test in z_tests:
assert isa_regex.match(test), f"{test} should match"

# S extensions
s_tests = ["RV32I_Smrnmi", "RV32I_Svnapot"]
for test in s_tests:
assert isa_regex.match(test), f"{test} should match"

# Vector extensions
v_tests = ["RV32I_Zve32x", "RV32I_Zvl32b"]
for test in v_tests:
assert isa_regex.match(test), f"{test} should match"


class TestRealWorldScenarios:
"""Test realistic ISA configurations."""

def test_common_configurations(self):
"""Test ISA strings from real projects."""
configs = [
"RV32I",
"RV32IMC",
"RV32IMAFD",
"RV64IMAFD",
"RV32I_Zicsr_Zifencei",
"RV64IG_Zicsr_Zifencei",
"RV32I_Zba_Zbb_Zbc_Zbs",
"RV64I_Zfh_Zfa",
"RV32I_Zve32x_Zvl32b",
]

for config in configs:
assert isa_regex.match(config), f"Config {config} should work"

def test_user_mistakes(self):
"""Test common user errors."""
mistakes = [
("RV32G", "G needs I or E"),
("RV32i", "Wrong case"),
("RV32I_zicsr", "Extension case"),
("RV32IZicsr", "Missing underscore"),
("RV32I_Zicsr_", "Trailing underscore"),
("rv32i", "All lowercase"),
]

for mistake, _ in mistakes:
assert not isa_regex.match(mistake), f"{mistake} should fail"


def test_can_import_regex():
"""Test regex imports correctly."""
from riscv_config.constants import isa_regex
assert isa_regex is not None
assert callable(isa_regex.match)