Skip to content
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

unit tests: do not fail when /etc/machine-id is empty #25732

Merged
merged 5 commits into from
Dec 15, 2022

Conversation

enr0n
Copy link
Contributor

@enr0n enr0n commented Dec 13, 2022

This is sort of a follow on to #25120, since some of these tests also fail when /etc/machine-id exists, but is empty. To simplify these checks, add a new test helper machine_id_initialized() which covers the cases when /etc/machine-id is empty, non-existent, or contains "uninitialized".

For one test failure (test-journal-interleaving), it seemed the appropriate resolution was to actually patch the source. The rest of the changes are in unit tests that should be at least partially skipped when /etc/machine-id has not been initialized.

@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label Dec 13, 2022
src/shared/tests.c Outdated Show resolved Hide resolved
src/libsystemd/sd-journal/journal-file.c Outdated Show resolved Hide resolved
src/shared/tests.c Outdated Show resolved Hide resolved
@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks tests and removed please-review PR is ready for (re-)review by a maintainer labels Dec 14, 2022
This also ensures that the test is skipped when /etc/machine-id exists,
but is not initialized.
…nitialized

The part of test_chase_symlink in test-fs-util that calls
sd_id128_get_machine will fail if /etc/machine-id is empty, so skip this
block if the machine-id is not initialized.
When executed on a systemd with an empty /etc/machine-id,
test-journal-interleaving fails in test_sequence_numbers_one() when
re-opening the existing "two.journal". This is because opening the
existing journal file with managed_journal_file_open() causes
journal_file_verify_header() to be called. This function tries to
compare the current machine-id to the machine-id in the journal file
header, but does not handle the case where the machine-id is empty or
non-existent.

Check if we have an initialized machine-id before executing this portion
of the test.
@enr0n enr0n force-pushed the unit-test-machine-id-initialized branch from 3ab5a48 to 3a9ca23 Compare December 14, 2022 19:03
@github-actions github-actions bot added journal please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Dec 14, 2022
@enr0n
Copy link
Contributor Author

enr0n commented Dec 14, 2022

I dropped the machine_id_initialized() helper in favor of using sd_id128_get_machine(NULL), and I modified test-journal-interleaving rather than modifying the source. PTAL.

@yuwata yuwata added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed journal please-review PR is ready for (re-)review by a maintainer labels Dec 14, 2022
@yuwata yuwata merged commit 60e84f0 into systemd:main Dec 15, 2022
@keszybz keszybz removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Dec 20, 2022
jackpot51 pushed a commit to pop-os/systemd that referenced this pull request May 7, 2024
…itialized.patch

The latest stable tag includes some, but not all patches which [1] needs
to work correctly. For now, just simplify the `machine_id_initialized()`
helper and make a note in the patch that the changes were forwarded
upstream.

[1] systemd/systemd#25732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants