Drop the %_dbpath override from our test-container #3525
Conversation
Somehow nobody has noticed we don't create the per-host configuration directory on install, probably didn't even in autoconf days. We'll need this now in the next commits though.
These /usr/lib/sysimage/rpm/pubkeys paths are valid for the OS rpm in the test-container, but that might not have any correlation to the *built* rpm we're supposed to be testing here. This is masked by the %_dbpath override in the Dockerfile which we'll be removing in the next step.
pmatilai
force-pushed
the
test-dbpath-pr
branch
from
995b4a3 to
68f9f4f
Compare
|
It's passing "make ci" locally but some difference on the GH runner .. sigh. This will have to wait for another day. |
tests/rpmdb.at
Outdated
| @@ -83,7 +81,7 @@ AT_KEYWORDS([rpmdb]) | |||
|
|
|||
| # This needs to run *without* RPMDB_INIT to test behavior on read-only mount | |||
| RPMTEST_CHECK([ | |||
| rpmdb --exportdb > rdonly.list | |||
| rpmdb --exportdb --dbpath /data/misc > rdonly.list | |||
There was a problem hiding this comment.
I can reproduce the CI failure with a make check locally here. I think this is the same case as previously - we need to initialize an empty database beforehand and only then make it read-only to actually test the behavior. Not (yet) sure how to do that here, though.
There was a problem hiding this comment.
Okay, it seems like using /var/lib/rpm-testsuite instead actually would make sense. Except we're then asserting that the file is not empty whereas it's of course empty since the rpmdb is 😄
I think this test just happened to work previously but it was never actually testing a truly read-only path.
There was a problem hiding this comment.
Nevermind, I've somehow missed that there's a prebuilt rpmdb in tests/data/misc directly in the repo, my bad.
There was a problem hiding this comment.
Yup, both "check" and "ci" pass for me locally. It was exactly like this when trying to get this test to work in the first place 😓
There was a problem hiding this comment.
It's strange indeed as, contrary to yours, both were failing for me consistently. A real head-scratcher. But, let's not worry about that here and now 😅
There was a problem hiding this comment.
Just to leave a trail of this head-scratcher, I ran into this earlier in 4a67189. So whatever it is, it's consistent 😆
|
Yay, we're down to "just" one failure, test 276 (the read-only db export test case, commented on above). |
| @@ -24,6 +24,12 @@ make_install() | |||
| mkdir -p $DESTDIR/build | |||
| ln -sf ../data/SOURCES $DESTDIR/build/ | |||
|
|
|||
| # setup an empty db that all tests are pointed to by default | |||
| dbpath="/var/lib/rpm-testsuite" | |||
There was a problem hiding this comment.
Should probably use local here (to avoid leaking the variable outside this bash function).
There was a problem hiding this comment.
I'm also thinking that for consistency, we could name this /var/lib/mktree or something (as there's already /usr/share/mktree for some helper scripts). But that's really the absolutely last thing to worry about here 😄
There was a problem hiding this comment.
I dislike the rpm-testsuite name too, but I do want it to have rpm in it 😅
There was a problem hiding this comment.
Yup, makes more sense after all 😄
There was a problem hiding this comment.
OTOH this is now centrally set, we only need to change that one line to move it elsewhere.
There was a problem hiding this comment.
Nah, I don't really care that much 😄 Let's keep it as is (whichever name works, really).
pmatilai
force-pushed
the
test-dbpath-pr
branch
from
5c92f6f to
a6c86c2
Compare
tests/rpmdb.at
Outdated
| # Assert that there are at least 3 packages in the rpmdb (we are using that of | ||
| # the test image here so that is pretty much guaranteed). | ||
| runroot rpm -qa | wc -l | xargs test 2 -lt || exit 1 | ||
| runroot rpm -qa | wc -l | xargs test 2 -eq || exit 1 |
There was a problem hiding this comment.
This was actually deliberately > 2 😅, so that we could test that filtering out the package names only shows those (and not any other). But meh...
There was a problem hiding this comment.
It's probably better to just install 3 packages instead of 2, I was just too lazy to do that I guess...
There was a problem hiding this comment.
Right, a subtlety I missed there. We can just toss more packages into the db, not a problem.
There was a problem hiding this comment.
Added two more packages to the install and test for the exact expected number of 'em in the db since we now can.
There was a problem hiding this comment.
Thanks, it's funny that that was easier than writing a whole comment line with an excuse 😆
There was a problem hiding this comment.
Yup, happens every now and then: one goes to great lengths trying to avoid a thing where actually just doing the it would be far, far easier 😅
Relying on the system db causes all sorts of headaches and irregularities that we're better off not having.
…iner This override exists to make system rpmdb accessible in eg "make shell" but it causes complications wrt the prefix (see below + next commits) and also masks test bugs (see previous commits). We can resurrect this functionality later on if we find solutions that avoid the issues. Instead of trying to inject the system rpmdb path into the test-image in a prefix-independent manner, set + create an empty, test-suite specific rpmdb path for all the tests to use by default. This dodges the pesky prefix question and accidental test-dependencies on system rpmdb. It should also allow us to optimize away a bunch of RPMDB_INIT's in the tests later on, but that's an exercise for another day. This is the first step to fixing rpm-software-management#3521.
pmatilai
force-pushed
the
test-dbpath-pr
branch
from
a6c86c2 to
9d0d213
Compare
This override exists to make system rpmdb accessible in eg "make shell" but it causes complications wrt the prefix (see below + next commits) and also masks test bugs (see previous commits). We can resurrect this functionality later on if we find solutions that avoid the issues.
Instead of trying to inject the system rpmdb path into the test-image in a prefix-independent manner, set + create an empty, test-suite specific rpmdb path for all the tests to use by default. This dodges the pesky prefix question and accidental test-dependencies on system rpmdb. It should also allow us to optimize away a bunch of RPMDB_INIT's
in the tests later on, but that's an exercise for another day.
This is the first step to fixing #3521.
The first commits are just pre-requisites for making this possible, and the change itself is a pre-requisite for fixing #3521.