Conversation
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
LecrisUT
added
test coverage
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds integration test cases (install-cases) and mock RPM spec files to test various package installation scenarios. Key feedback includes correcting the FMF context interpolation syntax by removing the leading $ from @{verified_artifact}, escaping $ in mktemp within rlRun, removing a redundant run directory creation, double-quoting the ${xfail_plans[@]} array expansion, and creating unique run directories inside the loop to prevent log overwriting.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| order: 40 | ||
| how: artifact | ||
| provide: | ||
| - file:rpms/$@{verified_artifact}/*.rpm |
There was a problem hiding this comment.
Remove the leading $ from the fmf context interpolation syntax. The correct syntax is @{verified_artifact}.
- file:rpms/@{verified_artifact}/*.rpm
|
|
||
| rlJournalStart | ||
| rlPhaseStartSetup "Prepare test environment" | ||
| rlRun "testdir=$(mktemp -d)" 0 "Create test directory" |
There was a problem hiding this comment.
Escape the $ in the mktemp command so that it is executed inside rlRun. This allows rlRun to properly check the exit status of mktemp and report a failure if it fails.
| rlRun "testdir=$(mktemp -d)" 0 "Create test directory" | |
| rlRun "testdir=\$(mktemp -d)" 0 "Create test directory" |
| rlRun "cp -a install-cases $testdir/data" 0 "Copy test data" | ||
| rlRun "cp -a rpms $testdir/data/" 0 "Copy rpms data" | ||
| rlRun "pushd $testdir/data" 0 "Enter test directory" | ||
| rlRun "run=$(mktemp -d)" 0 "Create run directory" |
There was a problem hiding this comment.
Remove this line as the run directory is now created uniquely per test case inside the loop.
| for plan in $(tmt plans ls); do | ||
| xfail="" | ||
| expected_result=0 | ||
| for check_pattern in ${xfail_plans[@]}; do |
There was a problem hiding this comment.
Double quote the array expansion ${xfail_plans[@]} to prevent word splitting and globbing.
| for check_pattern in ${xfail_plans[@]}; do | |
| for check_pattern in "${xfail_plans[@]}"; do |
| rlPhaseStartTest "$phase_prefix $plan $xfail" | ||
| rlRun "tmt run $extra_env -i $run --scratch -vvv --all \ | ||
| plan --name $plan \ | ||
| provision -h $PROVISION_HOW --image $image" \ | ||
| $expected_result "Run test case $plan $xfail" |
There was a problem hiding this comment.
Create a unique run directory for each test case inside the loop to preserve logs for debugging. Reusing a single $run directory with --scratch sequentially overwrites the logs of previous runs, making it very difficult to debug failures.
| rlPhaseStartTest "$phase_prefix $plan $xfail" | |
| rlRun "tmt run $extra_env -i $run --scratch -vvv --all \ | |
| plan --name $plan \ | |
| provision -h $PROVISION_HOW --image $image" \ | |
| $expected_result "Run test case $plan $xfail" | |
| rlRun "run_dir=\$(mktemp -d -p \"\$testdir\")" 0 "Create run directory" | |
| rlPhaseStartTest "$phase_prefix $plan $xfail" | |
| rlRun "tmt run $extra_env -i \$run_dir --scratch -vvv --all \ | |
| plan --name $plan \ | |
| provision -h $PROVISION_HOW --image $image" \ | |
| $expected_result "Run test case $plan $xfail" |
| SYSTEM_PACKAGE: foo-1.4 | ||
| EXPECTED_PACKAGE: "" | ||
| PRE_INSTALLED_PACKAGE: "" | ||
| DNF_CMD: dnf |
There was a problem hiding this comment.
I assume we will be also later testing wiht yum? Would it not be better to prepare for that here now?
| context: | ||
| want_devel: "false" | ||
| with_bar: "false" | ||
| xfail: "false" |
There was a problem hiding this comment.
I had to lookup what want_devel means, it would be good to drop here some comments so it is explained what these context variables are intended for, would definitely help me at lesat for the first one.
2 participants
Related to #4910, but does not cover all cases. For now the cases covered are:
/baseline: the bare minimum for package installation/verified-artifacts/basic: the equivalent of above but with verified artifacts/no-artifacts/upgrade: the failure case described in Installation of test dependencies is using inconsistent approach #4421/verified-artifacts/pre-installed: the failure case described in Artifact install plugin does not update an already installed package #4838(previously #4924, but that was closed because it was opened from a fork)