Validate only relevant parts of lvm+resize_root #9962

b10n1k · 2020-04-03T09:24:45Z

Validate only relevant parts of lvm+resize_root

Create scheduler with only relevant modules to validate the scenario
and make sure that the /root is resized.
The partitioning_resize_root.pm has been modified to use the Object_Oriented
framework. Minor changes added. A function to open the current suggestion is added
and the existing _run_expert_partitioner_ funtion is adjast to make available the choice
between the latest and the option of _the existing_partition_

Condition of _existing-partition_ and corresponding constant in the _select_item_in_system_view_table_
removed as it is not used and it does not provide any usability.

Related ticket: https://progress.opensuse.org/issues/64911
Needles: https://gitlab.suse.de/openqa/os-autoinst-needles-sles/-/merge_requests/1365
Verification run: http://aquarius.suse.cz/tests/2511

b10n1k · 2020-04-14T08:07:38Z

leave it open for reviews but i am planning to try improve some parts as to retain some Object-oriented principles (if it i can do so without too many cases). Also i wonder if i need to add a validation for the resize partition. I have checked only manual so far.

b10n1k · 2020-04-17T05:18:56Z

@OleksandrOrlov @JRivrain

rwx788

We don't have a module which asserts that size of the root partition is as per test data, which is one of the main differences to the default installation.

lib/Installation/Partitioner/ExpertPartitionerPage.pm

OleksandrOrlov · 2020-04-17T08:01:44Z

tests/installation/partitioning_resize_root.pm

+
+    my $partitioner = $testapi::distri->get_expert_partitioner();
+    $partitioner->run_expert_partitioner($test_data->{root}->{expert_partitioner_from});
+    $partitioner->resize_partition_on_gpt_disk($test_data->{root});
    send_key $cmd{accept};


Please, do not use os-autoinst functions in test module. https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/ui-framework-documentation.md#access-to-os-autoinst-testapi-from-test-module

There is already created method for the case: $partitioner->accept_changes();

I did make the change, adopting the accept_changes() but as you see i had to rename the current method to accept_changes_and_press_next() and create a new one. the reason is that the resize_partition_on_gpt_disk has already press_next() which the current accept_changes() calls it again after the press_accept_button();. i think it is more clear that trying to refactor part of the logic here, which is a bit tight between the connection of the methods.

b10n1k · 2020-04-21T11:18:46Z

We don't have a module which asserts that size of the root partition is as per test data, which is one of the main differences to the default installation.

the tickets implies to ensure that the resize is happening. i think this is covered + the shrinking to the relevant modules. are you asking for a validation of the partition size? i was asking for it here #9962 (comment).

b10n1k · 2020-04-22T17:24:16Z

@rwx788 i added validation at the end of the scheduler. I assume that this is what you were asking for.

rwx788 · 2020-04-23T06:33:12Z

@rwx788 i added validation at the end of the scheduler. I assume that this is what you were asking for.

I don't see any module which validates resizing and this is the main difference of this scenario to the default installation. We should assert that SUT got disk size as we have set it.

b10n1k · 2020-04-23T10:47:02Z

extra tests http://aquarius.suse.cz/tests/2512 and http://aquarius.suse.cz/tests/2513

OleksandrOrlov · 2020-04-23T11:04:39Z

I have a suggestion, which first of all will reduce the amount of changes in your PR and as the second - will not change existing API of AbstractExpertPartitioner.

Just remove partitioning_finish module from the schedule and return back accept_changes method. I do not see any real need in the partitioning_finish module to be added to the schedule, as it simply clicks 'Next' button.

It was added in the past, probably because our partitioner module was not so easy to understand. It should be removed from all our test suites where we'll use new approach.

Other than that LGTM.

b10n1k · 2020-04-23T16:14:56Z

I have a suggestion, which first of all will reduce the amount of changes in your PR and as the second - will not change existing API of AbstractExpertPartitioner.

Just remove partitioning_finish module from the schedule and return back accept_changes method. I do not see any real need in the partitioning_finish module to be added to the schedule, as it simply clicks 'Next' button.

It was added in the past, probably because our partitioner module was not so easy to understand. It should be removed from all our test suites where we'll use new approach.

Other than that LGTM.

The reason for this is that we have the chance to apply Open Closed principle here. Instead of rely on the scheduler we empower the framework. This might be useful in the future as to be depended to the scheduler is not much flexiple. As of the number of changes is small ones and easy to check for the other tests that they use them. So if it is ok i would like to keep those changes. let me know if it is not clear what i am saying

lib/Installation/Partitioner/Libstorage/ExpertPartitionerController.pm

lib/Installation/Partitioner/LibstorageNG/SuggestedPartitioningPage.pm

rwx788 · 2020-04-24T07:02:08Z

lib/Installation/Partitioner/LibstorageNG/SuggestedPartitioningPage.pm

+
+  select_start_with_existing_partitions()
+
+Opens current suggestion of the Expert Partiotioner from the Suggested Partitioning page .


It's actually current partitions, not suggestion. Don't you mind to improve this description?

i tried to use the term of the UI. one is existing_partition and the other is current_suggestion. change done

rwx788

Please, run VR for SLE 15 SP1 https://openqa.suse.de/tests/4142241 QR update image.

b10n1k · 2020-04-24T10:33:23Z

Please, run VR for SLE 15 SP1 https://openqa.suse.de/tests/4142241 QR update image.

Thanks for it as well. needles are differed and that was a nice catch. QA god bless you man

b10n1k · 2020-04-24T11:34:38Z

http://aquarius.suse.cz/tests/2538 passed. i have to add two more needles. i am not sure about the naming. should be distinguished for it somehow?

Please, run VR for SLE 15 SP1 https://openqa.suse.de/tests/4142241 QR update image.

rwx788 · 2020-04-24T11:37:41Z

http://aquarius.suse.cz/tests/2538 passed. i have to add two more needles. i am not sure about the naming. should be distinguished for it somehow?

Please, run VR for SLE 15 SP1 https://openqa.suse.de/tests/4142241 QR update image.

There are ENV-* tags you can add so needles are used for specific versions only, e.g. ENV-15SP2ORLATER-1.
But that's not mandatory to do.

Create scheduler with only relevant modules to validate the scenario and make sure that the /root is resized. The partitioning_resize_root.pm has been modified to use the Object_Oriented framework. Minor changes added. A function to open the current suggestion is added and the existing _run_expert_partitioner_ funtion is adjast to make available the choice between the latest and the option of _the existing_partition_. I renamed also the accept_changes and created a new one with the same name to make clear the separation of their functionality. Condition of _existing-partition_ and corresponding constant in the _select_item_in_system_view_table_ removed as it is not used and it does not provide any usability.

rwx788

Hmm, I don't see validate_modify_existing_partition and what worries me more that CI didn't catch it...

rwx788 · 2020-04-24T13:26:28Z

My bad, file is there, all good. We can merge.

b10n1k force-pushed the poo64911 branch from 1d2b1cb to 9ac8ebc Compare April 14, 2020 08:04

b10n1k force-pushed the poo64911 branch 6 times, most recently from 0a06dca to 6c88a1d Compare April 16, 2020 17:15

b10n1k changed the title ~~[WIP] Validate only relevant parts of lvm+resize_root~~ Validate only relevant parts of lvm+resize_root Apr 16, 2020

b10n1k force-pushed the poo64911 branch from 6c88a1d to 4b6736f Compare April 16, 2020 17:36

b10n1k marked this pull request as ready for review April 17, 2020 05:20

rwx788 reviewed Apr 21, 2020

View reviewed changes

OleksandrOrlov reviewed Apr 21, 2020

View reviewed changes

b10n1k force-pushed the poo64911 branch from 4b6736f to 9714a13 Compare April 22, 2020 16:21

b10n1k force-pushed the poo64911 branch 2 times, most recently from 86c9856 to e2f8cf1 Compare April 23, 2020 10:27

b10n1k requested review from rwx788 and OleksandrOrlov April 23, 2020 10:45

rwx788 reviewed Apr 24, 2020

View reviewed changes

lib/Installation/Partitioner/Libstorage/ExpertPartitionerController.pm Show resolved Hide resolved

rwx788 reviewed Apr 24, 2020

View reviewed changes

lib/Installation/Partitioner/LibstorageNG/SuggestedPartitioningPage.pm Outdated Show resolved Hide resolved

rwx788 reviewed Apr 24, 2020

View reviewed changes

b10n1k force-pushed the poo64911 branch from e2f8cf1 to abe1416 Compare April 24, 2020 09:56

b10n1k force-pushed the poo64911 branch from abe1416 to f268cbd Compare April 24, 2020 11:40

rwx788 reviewed Apr 24, 2020

View reviewed changes

rwx788 merged commit 715161b into os-autoinst:master Apr 24, 2020

b10n1k deleted the poo64911 branch August 24, 2023 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate only relevant parts of lvm+resize_root #9962

Validate only relevant parts of lvm+resize_root #9962

b10n1k commented Apr 3, 2020 •

edited

b10n1k commented Apr 14, 2020

b10n1k commented Apr 17, 2020

rwx788 left a comment

OleksandrOrlov Apr 17, 2020

b10n1k Apr 22, 2020

b10n1k commented Apr 21, 2020

b10n1k commented Apr 22, 2020

rwx788 commented Apr 23, 2020

b10n1k commented Apr 23, 2020

OleksandrOrlov commented Apr 23, 2020 •

edited

b10n1k commented Apr 23, 2020

rwx788 Apr 24, 2020

b10n1k Apr 24, 2020

rwx788 left a comment

b10n1k commented Apr 24, 2020

b10n1k commented Apr 24, 2020

rwx788 commented Apr 24, 2020

rwx788 left a comment

rwx788 commented Apr 24, 2020


		select_start_with_existing_partitions()

		Opens current suggestion of the Expert Partiotioner from the Suggested Partitioning page .

Validate only relevant parts of lvm+resize_root #9962

Validate only relevant parts of lvm+resize_root #9962

Conversation

b10n1k commented Apr 3, 2020 • edited

b10n1k commented Apr 14, 2020

b10n1k commented Apr 17, 2020

rwx788 left a comment

Choose a reason for hiding this comment

OleksandrOrlov Apr 17, 2020

Choose a reason for hiding this comment

b10n1k Apr 22, 2020

Choose a reason for hiding this comment

b10n1k commented Apr 21, 2020

b10n1k commented Apr 22, 2020

rwx788 commented Apr 23, 2020

b10n1k commented Apr 23, 2020

OleksandrOrlov commented Apr 23, 2020 • edited

b10n1k commented Apr 23, 2020

rwx788 Apr 24, 2020

Choose a reason for hiding this comment

b10n1k Apr 24, 2020

Choose a reason for hiding this comment

rwx788 left a comment

Choose a reason for hiding this comment

b10n1k commented Apr 24, 2020

b10n1k commented Apr 24, 2020

rwx788 commented Apr 24, 2020

rwx788 left a comment

Choose a reason for hiding this comment

rwx788 commented Apr 24, 2020

b10n1k commented Apr 3, 2020 •

edited

OleksandrOrlov commented Apr 23, 2020 •

edited