Make it possible to get new provides from RPM files#7
Merged
Conversation
hroncok
reviewed
May 5, 2026
hroncok
reviewed
May 5, 2026
hroncok
reviewed
May 5, 2026
Real source RPMs on Fedora report arch='noarch' in their RPM header, not 'src'. Checking arch == 'src' silently skips the detection and causes the SRPM to appear as a binary package in the provides list and verbose output. RPMTAG_SOURCEPACKAGE is the correct tag: it is set to a truthy value for source RPMs regardless of how the arch tag is reported. Update test_skip_source_rpm to reflect real SRPM behaviour (arch='noarch', sourcepackage=1) and add RPMTAG_SOURCEPACKAGE to the rpm patches in both unit and integration tests. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The previous code checked RPMSENSE_EQUAL first, which is a bitmask that is also set when the combined flag is >= (EQUAL|GREATER) or <= (EQUAL|LESS). This caused >= and <= constraints to be emitted as plain = in the provides string, silently misrepresenting the dependency. Check the combined EQUAL+GREATER and EQUAL+LESS cases before the individual EQUAL case so the correct operator string is produced. Add RPMSENSE_GREATER and RPMSENSE_LESS to the rpm patches in both unit and integration tests so the constants are well-defined during testing. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…utput The rpm Python bindings on Fedora (rpm >= 4.14) return str for header string fields, not bytes. The isinstance/decode guards on every field were dead code that added noise and made the mocks unnecessarily complex. Remove all .decode() / isinstance(..., bytes) calls and update MockRPMHeader and test data accordingly (no more b'...' literals in test fixtures). Compute evr once before rpm_info is populated and reuse it for the verbose "Reading ..." line, so the output omits the epoch when it is zero (matching the evr format already used in the rest of the output). Also use '7.1' instead of '7.1.0' in the provides version of the first integration test, which is more representative of what rpm actually reports for a package whose upstream version is '7.1'. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The 18-line patch block for rpm constants was duplicated across every test method in both the unit and integration test files. Extract it into an rpm_patches(mock_ts) context manager and switch all call sites to use it. Use parentheses instead of backslashes to wrap the multi-patch block inside the helper (PEP 8 preferred style). Also fix the unit test module docstring which incorrectly said "actual RPM files" when it always uses mocks. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The two --rpm-dir tests patched os.path.isdir and glob.glob directly, coupling the tests to implementation internals (the fact that the code uses os.path.isdir rather than pathlib, for example). Replace the mocked filesystem calls with real filesystem state: - test_cli_rpm_dir_not_found: pass tmp_path / 'nonexistent', which is a genuinely non-existent directory with no patching needed. - test_cli_rpm_dir_no_rpms: pass the empty tmp_path directly. Replace the try/except SystemExit pattern with pytest.raises(SystemExit) in all three tests, which is the idiomatic pytest style. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
The first four commits implement fixes for the issues you've identified. The last one takes it one step further by implementing real RPM packages for testing to replace heavy patching. There is also a new script to rebuild them all and source spec files so we can add more features/attributes to them for new tests. |
Member
|
The first 5 commits look awesome, thanks. Will eyeball the largest one in a bit. |
hroncok
approved these changes
May 11, 2026
hroncok
left a comment
hroncok
left a comment
Member
There was a problem hiding this comment.
One tiny non-bloking nit. Other than that, this is exactly what I had in mind.
Replace the heavily-patched MockRPMHeader approach in test_rpm_reading.py
and test_rpm_checking.py with actual RPM files built from minimal spec files.
The mock approach was fragile in two ways that the reviewer flagged:
- test_skip_source_rpm only passed because the mock hardcoded arch='src',
but real SRPMs have arch='noarch' — the test was not testing real behaviour.
- Every test carried 18 patch() calls that coupled tests to implementation
details (which constants the code reads, whether it uses bytes or str, etc.).
New layout:
tests/fixtures/rpms/
specs/ - five spec files, one per test scenario
rebuild.sh - regenerate RPMs from specs (requires rpmbuild)
*.noarch.rpm - six pre-built binary RPMs
*.src.rpm - one pre-built source RPM
Packages (all noarch, no dist tag for cross-Fedora stability):
revdeptest-foo 1.0 simple package, no epoch
revdeptest-bar 1.0 different SRPM (for mixed-SRPM error test)
revdeptest-bundled 1.0 has bundled(libfoo) and bundled(libbar) provides
revdeptest-multi 1.0 main package with a subpackage (same SRPM)
revdeptest-multi-sub 1.0 subpackage of revdeptest-multi
revdeptest-epoch 9.1.0 Epoch: 1 package
test_rpm_reading.py now imports nothing from unittest.mock and carries no
rpm-module patches. It passes real paths to read_rpm_provides() and asserts
on the real header values returned by the rpm library.
test_rpm_checking.py keeps MockBase/MockPackage for the DNF layer (which has
its own test coverage) but passes real RPM files into check_rpm_files().
Remove *.spec from .gitignore (added for PyInstaller; not relevant here and
was hiding the new spec files from git).
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The idea is to run
fedpkg mockbuildand then use the results folder as an input here, so the real provides and versions are used.