Hi - > And I don't like the pretty big testfiles, which aren't self-contained.
> You have to fetch them from a remote koji server. It would be much > better to have a (small) self-contained .spec file that can be used to > generate the rpms. Frank is already working on that. Well, one did not have to fetch them from anywhere, they were already there. But yes, working on a smaller synthetic set of RPMs. > > +debuginfod_build_id_find_LDADD = $(libdw) > > You also use libelf functions, they'll be pulled in through libdw, but > I think it is better to be explicit > debuginfod_build_id_find_LDADD = $(libelf) $(libdw) OK. > > +set -x > > Is this really necessary? > It makes the log files very verbose. > Or is that the intention? I've found it tricky to debug failing tests without this. > > +# find an unused port number > > +while true; do > > + PORT1=`expr '(' $RANDOM % 1000 ')' + 9000` > > + ss -atn | fgrep ":$PORT1" || break > > +done > > Which package does ss come from? iproute-5.2.0-1.fc30.x86_64 > Make sure it is listed as a BuildRequires. OK, because of %check in the RPM? > Also I am slightly worried about it not finding anything and looping > forever. Probably unlikely, but ss might misfunction? There is no 100%-probable way of assigning a port number ahead of time. Would you like an iteration limit / abort for that unlikely situation? > What happens if ss isn't installed? The set -e should nuke the test run. > If we assume bash (which I believe we already do in some places) you > could use the bash builtin special /dev/tcp redirection to test whether > a localhost port is open with echo >/dev/tcp/localhost/$PORT OK, will try that. > > +mkdir F R > > +tempfiles F R > > O, they are directories, then yeah, tempfiles isn't enough. > Maybe don't mark them as tempfiles then. OK. > > +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= > > ${abs_builddir}/../debuginfod/debuginfod -vvvv -d $DB \ > > +-p $PORT1 -t0 -g0 R F & > > +PID1=$! > > +sleep 3 > > Is there no better way to test the server has started? This one may not be needed. > > +export DEBUGINFOD_URLS=http://localhost:$PORT1/ # or without trailing / > > + > > +# We use -t0 and -g0 here to turn off time-based scanning & grooming. > > +# For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process. > > + > > +######################################################################## > > + > > +# Compile a simple program, strip its debuginfo and save the build-id. > > +# Also move the debuginfo into another directory so that elfutils > > +# cannot find it without debuginfod. > > +echo "int main() { return 0; }" > ${PWD}/prog.c > > +tempfiles prog.c > > +gcc -g -o prog ${PWD}/prog.c > > + ${abs_top_builddir}/src/strip -g -f prog.debug ${PWD}/prog > > +BUILDID=`env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../src/readelf \ > > + -a prog | grep 'Build ID' | cut -d ' ' -f 7` > > + > > +mv prog F > > +mv prog.debug F > > +kill -USR1 $PID1 > > +sleep 3 # give enough time for scanning pass > > Hmm that hardcoded sleep 3 again :{ Well, here's the problem. We want to assert that the scan was successful. We can't just reject a 404 response because the scan may not have finished yet. So we'd have to race/loop/poll. But then we'd need a timeout (how long?). It turns out to be the same thing, just more complicated. > > +mv testfile-debuginfod-0.rpm R > > +mv testfile-debuginfod-1.rpm R > > +mv testfile-debuginfod-2.rpm R > > +kill -USR1 $PID1 > > +sleep 10 > > Why 10? To give extra time for scanning RPMs. > > +kill -USR1 $PID1 # two hits of SIGUSR1 may be needed to resolve > > .debug->dwz->srefs > > +sleep 10 > > And another :{ Yes, again, same reasons as above. You can either have a timeout-poll loop, or a time sleep and a single judgement poll. > > +# Trigger a cache clean and run the tests again. The clients should be > > unable to > > +# find the target. > > +echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s > > +echo 0 > $DEBUGINFOD_CACHE_PATH/max_unused_age_s > > + > > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1 > > + > > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID2 > > && false || true > > OK. But that means zero means never cache/always clean? > I would have expected 0 to mean "forever". I see the man page doesn't specifically disclose the interpretation of zero. A "no retention of prior results" purpose is useful, and is consistent with 0 as per the text. A "retain forever" setting would have to be a different value. - FChE