Skip to content

Conversation

kolyshkin
Copy link
Collaborator

@kolyshkin kolyshkin commented Aug 11, 2025

Please see individual commits for details.

High level overview:

  • move container setup logic to a script
  • make apt-get install less verbose
  • install known go version in containers
  • go.mod: bump go to 1.23
  • TestListJobs: fix flakiness
  • login1: fix TestListSessions vs new systemd
  • sdjournal: don't use fmt.Printf from tests
  • djournal/TestJournalGetCatalog: fix vs new systemd
  • add more images (newer ubuntu, debian, fedora)

These are moved to a followup PR:

  • Replace interface{} with any
  • Use for range for integers
  • sdjournal/journal_test.go: use slices.Contains
  • login1: remove unused method
  • sdjournal,machine1: don't use rand.Seed in test
  • Remove io/ioutil usage

@kolyshkin kolyshkin requested a review from Luap99 August 11, 2025 18:02
Copy link
Collaborator

@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.

Thanks, that all makes sense

Looks like the container test all ship an outdated go? I guess from the normal LTS repos. Likely need to sideload go there.

@kolyshkin kolyshkin changed the title go.mod: bump go to 1.23, modernize [DNM] go.mod: bump go to 1.23, modernize Aug 11, 2025
@kolyshkin kolyshkin force-pushed the go123 branch 11 times, most recently from 2709a7d to fe1e568 Compare August 11, 2025 21:35
Move the logic of running tests in a container to the script,
as it is easy to work with.

No change in functionality, except for shorter sleep duration.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin kolyshkin force-pushed the go123 branch 7 times, most recently from be69c8f to 128647a Compare August 12, 2025 19:30
Instead of relying a distro to provide a recent Go version, let's
install it ourselves from a binary distribution.

Signed-off-by: Kir Kolyshkin <[email protected]>
Currently, go 1.23 is the "old" supported version, meaning the older
versions are no longer supported.

Let's use it as a minimally required version.

Signed-off-by: Kir Kolyshkin <[email protected]>
In some cases, TestListJobs fails like this:

	methods_test.go:712: oneshot.service job not found in list

This happens because 'ExecStart=/bin/sh sleep 400' doesn't really run
sleep; it merely fails. So, the job is queued and then ExecStart fails
instantly, and if the test checks the job status after the failure, the
test fails.

Fix the ExecStart.

While at it,
 - fix unit description;
 - remove ExecStartPost as it's useless (and also wrong).

Signed-off-by: Kir Kolyshkin <[email protected]>
On newer distros, some session paths have c instead of _.

Fix the test accordingly.

Signed-off-by: Kir Kolyshkin <[email protected]>
The output also missed a newline.

Signed-off-by: Kir Kolyshkin <[email protected]>
This test assumed the first entry in systemd-journald.service log comes
with MESSAGE_ID set. This is not true for modern distros, so the test
fails.

Add a loop to scan for MESSAGE_ID, and skip if not found.

Co-authored-by: Claude Code
Signed-off-by: Kir Kolyshkin <[email protected]>
1. Replace ubuntu 22.04 with 24.04

2. Add debian trixie (13, "stable").

3. Add fedora.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin kolyshkin changed the title [DNM] go.mod: bump go to 1.23, modernize [DNM] go.mod: bump go to 1.23, fix tests, add more images Aug 12, 2025
@kolyshkin kolyshkin changed the title [DNM] go.mod: bump go to 1.23, fix tests, add more images Bump go to 1.23, fix tests, add more images Aug 12, 2025
@kolyshkin kolyshkin requested a review from Luap99 August 12, 2025 19:49
@kolyshkin kolyshkin marked this pull request as ready for review August 12, 2025 19:52
Copy link
Collaborator

@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.

LGTM

@Luap99 Luap99 merged commit 66963a1 into coreos:main Aug 12, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants