Skip to content

kola/tests/update: add test for idle status#183

Closed
tormath1 wants to merge 2 commits intoflatcar-masterfrom
tormath1/update-engine
Closed

kola/tests/update: add test for idle status#183
tormath1 wants to merge 2 commits intoflatcar-masterfrom
tormath1/update-engine

Conversation

@tormath1
Copy link
Copy Markdown
Contributor

@tormath1 tormath1 commented Jul 2, 2021

this test assert that there is no regression regarding the status
check when there is no new upload available

Signed-off-by: Mathieu Tortuyaux mathieu@kinvolk.io

Related to: flatcar/update_engine#10

this test assert that there is no regression regarding the status
check when there is no new upload available

Signed-off-by: Mathieu Tortuyaux <mathieu@kinvolk.io>
@tormath1 tormath1 self-assigned this Jul 2, 2021
@tormath1 tormath1 marked this pull request as ready for review July 2, 2021 14:46
c.Fatalf("unable to run update_engine_client -update: %v", err)
}

if string(out) == "Update cancelled -- remote version is not newer than the local one" {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm new to this, but just reading the logic I would think that the test should be:

Suggested change
if string(out) == "Update cancelled -- remote version is not newer than the local one" {
if string(out) != "Update cancelled -- remote version is not newer than the local one" {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jepio that's totally right, it seems my 🧠 is already in week-end. :D

I also added an output redirection in the fixup commit.

// assert that -update does not fail
// when there is nothing to do
// https://github.com/kinvolk/Flatcar/issues/356
func idle(c cluster.TestCluster) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test is only valid for developer builds or as long as the next release is not published. It may be confusing when running the tests for a published release.

Besides that I'm also a bit concerned about the runtime of the tests because this small test needs its own instance created and that takes multiple minutes depending on the platform.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there is some support in kola for emulating an update service and to perform an update. In combination with this it makes sense to have a test where maybe in one test case an update is handed and in the other test case there is none. But I wouldn't let the behavior depend on the public update service.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok then we can close this PR for the moment - I'm currently trying to think about a better way to test the update behavior of Flatcar :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see that we are already in the right file: cl.update.payload is currently not used in Jenkins, would make sense to set this up.

--- SKIP: cl.update.payload (19.31s)
        update.go:93: no update payload provided

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah I dug around a bit - we would need to generate an update payload with delta_generator and to provide it to the kola command line. I can open a dedicated issue to track this :) That would be a good start !

@tormath1 tormath1 closed this Jul 2, 2021
@tormath1 tormath1 deleted the tormath1/update-engine branch July 2, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants