Skip to content

Conversation

@baude
Copy link
Member

@baude baude commented Nov 10, 2025

In tests that do not start a machine, we can use "fake" images to speed up tests. In the case of darwin and Linux, that can be /dev/null. The hypervisors don't care.

In the case of Windows, some research will need to be done to determine the same approach but this is a start.

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2025
@baude baude marked this pull request as draft November 10, 2025 15:27
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 10, 2025
@baude baude force-pushed the speedLinux branch 2 times, most recently from 5031ea4 to b8955e2 Compare November 10, 2025 16:12
In tests that do not start a machine, we can use "fake" images to speed
up tests.  In the case of darwin and Linux, that can be /dev/null.  The
hypervisors don't care.

In the case of Windows, some research will need to be done to determine
the same approach but this is a start.

Signed-off-by: Brent Baude <bbaude@redhat.com>
@timcoding1988
Copy link
Collaborator

LGTM

@mheon
Copy link
Member

mheon commented Nov 10, 2025

+1 from me - we are having serious timing issues with these tests.
LGTM

@baude baude marked this pull request as ready for review November 10, 2025 19:00
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 10, 2025
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care to much, I am fine merging as is but I think a a small doc on when/how to use it would help contributors understand when to use it.

If you like to merge as is feel free to drop the hold

/lgtm
/hold

fakeImagePath string = os.DevNull
)

func (i *initMachine) withFakeImage(_ *machineTestBuilder) *initMachine {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor style nits, given this is the same for everything except windows it would be less duplication to create a new file config_fakeimage.go that uses //go:build !windows as build tag

Also it would be good to add a function comment as documentation what this is for, i.e. when to use this in tests.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2025
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2025
@baude
Copy link
Member Author

baude commented Nov 11, 2025

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 11, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit f23367f into containers:main Nov 11, 2025
49 checks passed
baude added a commit to baude/podman that referenced this pull request Nov 11, 2025
this pr is a follow on to containers#27493.  it adds support for hyperv "fake"
images and suggests a benefit in terms of test speed.  for hyperv, we
create a generic 4MB vhdx and stick it into the temp dir.  this saves us
from any image copy or compression.

i also followed up on a few comments Paul made about using windows|unix
instead of each platform.

Signed-off-by: Brent Baude <bbaude@redhat.com>
@baude baude mentioned this pull request Nov 11, 2025
6 tasks
baude added a commit to baude/podman that referenced this pull request Nov 11, 2025
this pr is a follow on to containers#27493.  it adds support for hyperv "fake"
images and suggests a benefit in terms of test speed.  for hyperv, we
create a generic 4MB vhdx and stick it into the temp dir.  this saves us
from any image copy or compression.

i also followed up on a few comments Paul made about using windows|unix
instead of each platform.

Signed-off-by: Brent Baude <bbaude@redhat.com>
baude added a commit to baude/podman that referenced this pull request Nov 11, 2025
this pr is a follow on to containers#27493.  it adds support for hyperv "fake"
images and suggests a benefit in terms of test speed.  for hyperv, we
create a generic 4MB vhdx and stick it into the temp dir.  this saves us
from any image copy or compression.

i also followed up on a few comments Paul made about using windows|unix
instead of each platform.

Signed-off-by: Brent Baude <bbaude@redhat.com>
Luap99 added a commit to Luap99/libpod that referenced this pull request Nov 12, 2025
Two PRs[1,2] were merged without rebasing resulting in a conflict since
the one enabled gofumpt while the other PR contained formatting not
according to that so now the lint fails.

[1] containers#27498
[2] containers#27493

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99 Luap99 mentioned this pull request Nov 12, 2025
6 tasks
baude added a commit to baude/podman that referenced this pull request Nov 12, 2025
this pr is a follow on to containers#27493.  it adds support for hyperv "fake"
images and suggests a benefit in terms of test speed.  for hyperv, we
create a generic 4MB vhdx and stick it into the temp dir.  this saves us
from any image copy or compression.

i also followed up on a few comments Paul made about using windows|unix
instead of each platform.

Signed-off-by: Brent Baude <bbaude@redhat.com>
baude added a commit to baude/podman that referenced this pull request Nov 12, 2025
this pr is a follow on to containers#27493.  it adds support for hyperv "fake"
images and suggests a benefit in terms of test speed.  for hyperv, we
create a generic 4MB vhdx and stick it into the temp dir.  this saves us
from any image copy or compression.

i also followed up on a few comments Paul made about using windows|unix
instead of each platform.

Signed-off-by: Brent Baude <bbaude@redhat.com>
baude added a commit to baude/podman that referenced this pull request Nov 12, 2025
this pr is a follow on to containers#27493.  it adds support for hyperv "fake"
images and suggests a benefit in terms of test speed.  for hyperv, we
create a generic 4MB vhdx and stick it into the temp dir.  this saves us
from any image copy or compression.

i also followed up on a few comments Paul made about using windows|unix
instead of each platform.

Signed-off-by: Brent Baude <bbaude@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. machine release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants