Skip to content

Conversation

@ZarIliv
Copy link
Collaborator

@ZarIliv ZarIliv commented Oct 16, 2025

Enhance Project Infrastructure Setup

This PR modernizes SmartBugs' development infrastructure with comprehensive testing, type safety, and code quality tooling.

Key Changes:

  • Testing Infrastructure: Added pytest configuration with 8 core module test suites achieving 95%+ average coverage (820+ tests across docker, parsing, sarif, settings, smartbugs,
    solidity, tasks, tools modules)
  • Type Safety: Added comprehensive type hints across entire codebase with strict mypy enforcement
  • Code Quality: Integrated Black (formatting), Ruff (linting), and pre-commit hooks
  • Development Tools: Added Poetry support, comprehensive Makefile with common dev commands (make test, make lint, make format, make check)
  • Python Support: Dropped Python 3.8 (EOL), updated minimum to Python 3.9+
  • Test Fixtures: Added comprehensive test contracts and utilities for consistent testing

Files Changed: 122 files (+11,951, -1,318 lines)

Testing:

  • All existing functionality preserved with stable parser interfaces
  • Comprehensive unit test coverage validates core behavior
  • Pre-commit hooks ensure code quality standards

This establishes a solid foundation for future development with improved maintainability and code reliability.

… with poetry, linting, formatting and type checking configuration, pre-commit config, Makefile, documentation updates.

chore(python): Drop python 3.8 support, because it is EOL
fix(solidity): Convert Path object to string in solc version cache
fix(smartbugs): Convert Exception to string before appending to errors list
@ZarIliv ZarIliv requested a review from gsalzer October 27, 2025 09:45
Copy link
Member

@gsalzer gsalzer left a comment

Choose a reason for hiding this comment

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

On Ubuntu, after running install/setup-venv.sh --dev in the top folder of SB, the folder contains four empty files named =2.1.0, =3.11.1, =4.1.0 and =7.4.0.

@gsalzer
Copy link
Member

gsalzer commented Oct 31, 2025

make test lists 353 passed tests, and 2 skipped. For the two skipped tests, I get the error

SKIPPED [1] tests/unit/test_sarif.py:797: could not import 'jsonschema': No module named 'jsonschema'
SKIPPED [1] tests/unit/test_sarif.py:823: could not import 'jsonschema': No module named 'jsonschema'

For make all, I get the error

Linting code with Ruff...
/bin/sh: 2: venv/bin/ruff: not found
make: *** [Makefile:137: lint] Error 127

After doing pip install ruff, I get the error

Type checking with Mypy...
/bin/sh: 1: venv/bin/mypy: not found

After pip install mypy, I get the message

Type checking with Mypy...
There are no .py[i] files in directory 'tools'
Note: Mypy may report errors. Consider adding type hints gradually.
Type checking complete!

Is this ok?

Should we add pip install ruff mypy to the install script?

@jff
Copy link
Member

jff commented Nov 3, 2025

Thank you, @ZarIliv, excellent work!

On my side (macOS 14.7.6), after running make install it all seems to be properly installed, but I get the following error:

Installing the current project: smartbugs (2.0.15)
✓ Dependencies installed via Poetry

Verifying installation...
make: *** [install] Error 1

Looking at install/setup-venv.sh it seems that I should get more details on what check failed.

If I run install/setup-venv.sh --dev directly, I get no errors. But I also don't get the checking messages (the output terminates with Verifying installation...).

@ZarIliv
Copy link
Collaborator Author

ZarIliv commented Nov 3, 2025

Hi @jff,

Thanks for the review!

Unfortunately I do not have a system with macOS at my disposal, but from what you provided it seems that the install script fails at the line

python3 -c "import yaml; import colorama; import requests; import semantic_version; import docker; import cpuinfo" 2>/dev/null

This uses the python Installation of the OS instead of the created venv. I fixed these lines in my latest commit. Can you verify that it is working now?

@gsalzer: As already discussed: Your mentioned issues are fixed now.

@ZarIliv ZarIliv self-assigned this Nov 3, 2025
@gsalzer
Copy link
Member

gsalzer commented Nov 3, 2025

Still two installation issues, when installing with --dev.

  • Installation: generates four empty files =2.1.0, =3.11.1, =4.1.0 and =7.4.0. Probably caused by the line

    pip install pytest>=7.4.0 pytest-cov>=4.1.0 pytest-mock>=3.11.1 pytest-timeout>=2.1.0 --quiet

    The character > redirects the output to the file named after >, here the version numbers.

  • When running the test (make test), I still get

    ===================================================== short test summary info =====================================================
    SKIPPED [1] tests/unit/test_sarif.py:797: could not import 'jsonschema': No module named 'jsonschema'
    SKIPPED [1] tests/unit/test_sarif.py:823: could not import 'jsonschema': No module named 'jsonschema'
    ================================================= 353 passed, 2 skipped in 1.32s ==================================================
    

@jff
Copy link
Member

jff commented Nov 3, 2025

I now understand the issue I'm having: I have poetry installed and it is installing the dependencies in .venv instead of venv.

This is odd because poetry should be able to detect and use the virtual environment currently activated.

I can't look at the details and fix this right now. I'll try to do it later today.

@ZarIliv
Copy link
Collaborator Author

ZarIliv commented Nov 4, 2025

@gsalzer: Now I get the issue. The problem is, that on your machine poetry is not installed. Therefore the script uses the pip default path and installs dependencies based on the requirements file for your specific python version. We could either replace this section and always install poetry as well, or we could edit all requirements files (it would be a manual workflow, because we need to keep the requirements files and the pip install line consistent with our pyproject.toml file). I'd prefer to install poetry in the setup-venv.sh script.

@jff
Copy link
Member

jff commented Nov 4, 2025

Can I suggest that we create the folder .venvrather than venv? That is the default for poetry and, in my case, it always creates .venv rather than using venv. I am using Poetry v. 2.2.1

I changed the directory name to .venv and it all worked fine in my machine.

@gsalzer
Copy link
Member

gsalzer commented Nov 4, 2025

@ZarIliv We have currently three installation options: poetry, pip+requirements files, and pip+packages on the command line. This seems a bit too much.
The idea was to make installation as simple and robust/reproducible as possible. I'm fine with a pure poetry install if this fits the bill.
@jff I have no preferences regarding venv vs .venv. If .venv works better, let's use it.

@gsalzer
Copy link
Member

gsalzer commented Nov 4, 2025

@ZarIliv Is poetry a system package dependency, like docker, or can it be installed in the virtual environment? Requiring users of SmartBugs to install a system package just for installation purposes seems to be heavy.

@ZarIliv
Copy link
Collaborator Author

ZarIliv commented Nov 4, 2025

Poetry can be installed in the venv as well. I'll update the script accordingly.

@ZarIliv ZarIliv requested review from gsalzer and jff November 4, 2025 11:15
@jff
Copy link
Member

jff commented Nov 4, 2025

It's all working fine on my side! Thanks, @ZarIliv !

@gsalzer gsalzer merged commit 786ec64 into master Nov 5, 2025
2 checks passed
@gsalzer gsalzer deleted the Enhance-project-infrastructure-setup branch November 5, 2025 09:55
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.

4 participants