Bring up previous session #487

Jedoku · 2024-10-07T01:21:19Z

WIP

bin/auto_hck

lib/project.rb

lib/setupmanagers/qemuhck/qemu_machine.rb

akihikodaki · 2024-10-10T12:38:06Z

I'm not a fan of this idea. It brings significant complexity similar to --dump, which adds an alternative way to run tests.
Its use case is also unclear; I often repeatedly run tests using the command line history and I'm satisfied with it.

lib/session.rb

lib/setupmanagers/qemuhck/qemu_machine.rb

lib/engines/hcktest/playlist.rb

lib/engines/hcktest/tools.rb

lib/project.rb

lib/session.rb

lib/models/session.rb

lib/session.rb

lib/cli.rb

lib/session.rb

lib/models/session.rb

lib/engines/hcktest/hcktest.rb

lib/project.rb

lib/session.rb

lib/models/session.rb

lib/project.rb

lib/setupmanagers/qemuhck/qemu_machine.rb

akihikodaki · 2025-01-30T09:19:51Z

lib/engines/hcktest/hcktest.rb

@@ -274,7 +281,7 @@ def run_tests_with_config
        next if tests.empty?

        ResourceScope.open do |scope|
-          run_clients_and_configure_setup(scope, group => true, create_snapshot: false, boot_from_snapshot: true)
+          run_clients_and_configure_setup(scope, create_snapshot: false, group => true)


Why did you change this?

lib/engines/hcktest/hcktest.rb

lib/session.rb

lib/cli.rb

akihikodaki

The current implementation will leave an immature session if an error happens before clients get configured. Save the session after it is fully initialized (i.e., all clients are configured).

lib/cli.rb

Jedoku · 2025-01-31T10:01:15Z

The current implementation will leave an immature session if an error happens before clients get configured. Save the session after it is fully initialized (i.e., all clients are configured).

But this is one of features, you can bring up VM`s for checking what's goin on and where it's crash

kostyanf14 · 2025-01-31T10:36:06Z

lib/cli.rb

+      prop :svvp, T.nilable(T::Boolean)
+      prop :dump, T.nilable(T::Boolean)
+      prop :gthb_context_prefix, T.nilable(String)
+      prop :gthb_context_suffix, T.nilable(String)
+      prop :playlist, T.nilable(String)
+      prop :select_test_names, T.nilable(String)
+      prop :reject_test_names, T.nilable(String)
+      prop :reject_report_sections, T::Array[String], default: []
+      prop :boot_device, T.nilable(String)
+      prop :allow_test_duplication, T.nilable(T::Boolean)
+      prop :manual, T.nilable(T::Boolean)
+      prop :package_with_playlist, T.nilable(T::Boolean)


Don't see any reason to make boolean fields nilable. String or Array can be nilable because we have logic to check is field specified or not. But in the case of a boolean, we always check true or false.

akihikodaki · 2025-02-01T05:42:34Z

The current implementation will leave an immature session if an error happens before clients get configured. Save the session after it is fully initialized (i.e., all clients are configured).

But this is one of features, you can bring up VM`s for checking what's goin on and where it's crash

The current code will make tests fail in an unpredictable manner instead of reproducing the past failure. It skips creating snapshots and configuration when restored so if some failure has happened in these operations, it will not reproduce the failure, but it will run the following tests with an immature configuration and pose different symptoms that are not useful to debug.

Jedoku · 2025-02-01T10:15:29Z

It runs saved snapshots, and our VM's will be configured exactly as it was, also on we can open studio and see what happens in previous session

…

On Sat, Feb 1, 2025 at 7:42 AM Akihiko Odaki ***@***.***> wrote: The current implementation will leave an immature session if an error happens before clients get configured. Save the session after it is fully initialized (i.e., all clients are configured). But this is one of features, you can bring up VM`s for checking what's goin on and where it's crash The current code will make tests fail in an unpredictable manner instead of reproducing the past failure. It skips creating snapshots and configuration when restored so if some failure has happened in these operations, it will not reproduce the failure, but it will run the following tests with an immature configuration and pose different symptoms that are not useful to debug. — Reply to this email directly, view it on GitHub <#487 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANRORQZ7QAQGDXSBTNF6QMT2NRNGBAVCNFSM6AAAAABPO3NUC2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRYG44TAMZYGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

akihikodaki · 2025-02-01T20:07:09Z

It runs saved snapshots, and our VM's will be configured exactly as it was, also on we can open studio and see what happens in previous session

At least the session should be saved at least after snapshots are created, and if the VMs died before configuration, we should configure them before continuing to run tests.

The studio should have seen almost no activity at the early stage of execution before running tests; if it experienced a failure at the moment, it means the VM is so broken that it cannot boot or stay running for minutes. Such a scenario should be rare.

The easiest way to ensure the VMs are configured is simply to save the session after the configuration. It is sufficient to reproduce failures in tests.

lib/project.rb

akihikodaki · 2025-02-06T04:59:56Z

lib/session.rb

+    def self.load_session(cli)
+      session = Models::Session.from_json_file("#{session_path(cli)}/session.json")
+
+      session.common.workspace_path = cli.common.workspace_path


I don't think it is necessary.

lib/session.rb

lib/models/session.rb

lib/project.rb

lib/setupmanagers/qemuhck/qemu_machine.rb

lib/project.rb

Signed-off-by: Vitalii Chulak <vitalii@daynix.com>

akihikodaki · 2025-02-12T12:11:28Z

lib/setupmanagers/qemuhck/qemuhck.rb

@@ -45,7 +45,8 @@ def studio_vm_options
        'image_name' => @platform['st_image'],
        'logger' => @logger,
        'iso_path' => @project.config['iso_path'],
-        'share_on_host_path' => @project.options.common.share_on_host_path
+        'share_on_host_path' => @project.options.common.share_on_host_path,
+        'configured' => @project.restored?


This change should be in HCKTest as HCKTest coordinates to start VMs that are already configured. This logic is not valid for e.g., HCKInstall.

akihikodaki · 2025-02-12T12:25:06Z

lib/cli.rb

    end

    def parse(args)
      left = @parser.order(args)
      @mode = left.shift
      @sub_parser[@mode]&.order!(left) unless @mode.nil?
+
+      restore_session if @test.latest_session || @test.session


Instead of passing self to AutoHCK::Session.load and letting it modify, let's pass the path to the session.
Such a method may be called from bin/auto_hck and return an instance of loaded CLI, which will replace the whole existing CLI instance.

So instead of having these lines in session.rb:

cli.test = session.test cli.common = session.common

...add a line like the following in auto_hck:

cli = Session.load(session_path)

You don't have to list each attribute of CLI this way.

github-advanced-security bot found potential problems Oct 7, 2024

View reviewed changes

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch 2 times, most recently from a94ca1f to 71d5c6c Compare October 8, 2024 22:29

Jedoku requested review from akihikodaki and kostyanf14 October 8, 2024 22:52

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from 71d5c6c to 7e1ff8a Compare October 18, 2024 15:48

akihikodaki requested changes Oct 19, 2024

View reviewed changes

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from 7e1ff8a to 8a79a7e Compare November 27, 2024 13:14

Jedoku requested a review from akihikodaki December 1, 2024 00:10

Jedoku changed the title ~~RFC: Bring up previous session~~ Bring up previous session Dec 2, 2024

akihikodaki requested changes Dec 3, 2024

View reviewed changes

lib/project.rb Outdated Show resolved Hide resolved

lib/session.rb Outdated Show resolved Hide resolved

lib/models/session.rb Outdated Show resolved Hide resolved

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from 8a79a7e to 0ba68bc Compare December 19, 2024 08:54

Jedoku requested a review from akihikodaki December 19, 2024 09:06

akihikodaki requested changes Dec 19, 2024

View reviewed changes

lib/session.rb Outdated Show resolved Hide resolved

lib/session.rb Outdated Show resolved Hide resolved

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch 2 times, most recently from 72f53ca to dd5ec70 Compare January 22, 2025 09:01

Jedoku requested a review from akihikodaki January 22, 2025 11:17

akihikodaki requested changes Jan 23, 2025

View reviewed changes

kostyanf14 reviewed Jan 23, 2025

View reviewed changes

lib/setupmanagers/qemuhck/qemu_machine.rb Outdated Show resolved Hide resolved

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch 2 times, most recently from a63b74b to 0f480a0 Compare January 30, 2025 03:51

Jedoku requested review from kostyanf14 and akihikodaki January 30, 2025 03:53

akihikodaki requested changes Jan 30, 2025

View reviewed changes

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from 0f480a0 to df1cc61 Compare January 31, 2025 00:09

akihikodaki requested changes Jan 31, 2025

View reviewed changes

lib/cli.rb Outdated Show resolved Hide resolved

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from df1cc61 to 1684f10 Compare January 31, 2025 10:02

kostyanf14 reviewed Jan 31, 2025

View reviewed changes

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch 2 times, most recently from 606c9ec to e7b60b5 Compare January 31, 2025 11:03

Jedoku requested review from kostyanf14 and akihikodaki January 31, 2025 13:32

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from e7b60b5 to d0a7e85 Compare February 6, 2025 00:53

akihikodaki requested changes Feb 6, 2025

View reviewed changes

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from d0a7e85 to c746b95 Compare February 11, 2025 01:52

Jedoku requested a review from akihikodaki February 11, 2025 01:56

RHELMISC-4745: Add ability to bring up previous session

af3e26e

Signed-off-by: Vitalii Chulak <vitalii@daynix.com>

Jedoku force-pushed the Add-ability-to-bring-up-previous-session branch from c746b95 to af3e26e Compare February 11, 2025 02:44

akihikodaki requested changes Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bring up previous session #487

Bring up previous session #487

Jedoku commented Oct 7, 2024 •

edited

Loading

akihikodaki commented Oct 10, 2024

akihikodaki Jan 30, 2025

akihikodaki left a comment

Jedoku commented Jan 31, 2025

kostyanf14 Jan 31, 2025

akihikodaki commented Feb 1, 2025

Jedoku commented Feb 1, 2025 via email

akihikodaki commented Feb 1, 2025

akihikodaki Feb 6, 2025

akihikodaki Feb 12, 2025

akihikodaki Feb 12, 2025

Bring up previous session #487

Are you sure you want to change the base?

Bring up previous session #487

Conversation

Jedoku commented Oct 7, 2024 • edited Loading

akihikodaki commented Oct 10, 2024

akihikodaki Jan 30, 2025

Choose a reason for hiding this comment

akihikodaki left a comment

Choose a reason for hiding this comment

Jedoku commented Jan 31, 2025

kostyanf14 Jan 31, 2025

Choose a reason for hiding this comment

akihikodaki commented Feb 1, 2025

Jedoku commented Feb 1, 2025 via email

akihikodaki commented Feb 1, 2025

akihikodaki Feb 6, 2025

Choose a reason for hiding this comment

akihikodaki Feb 12, 2025

Choose a reason for hiding this comment

akihikodaki Feb 12, 2025

Choose a reason for hiding this comment

Jedoku commented Oct 7, 2024 •

edited

Loading