Skip to content

test/install: fix test for undefined behavior in slot selection handling#476

Merged
ejoerns merged 2 commits intorauc:masterfrom
ejoerns:topic/further-fixes-for-testing
Sep 11, 2019
Merged

test/install: fix test for undefined behavior in slot selection handling#476
ejoerns merged 2 commits intorauc:masterfrom
ejoerns:topic/further-fixes-for-testing

Conversation

@ejoerns
Copy link
Copy Markdown
Member

@ejoerns ejoerns commented Sep 9, 2019

For the triple-redundancy rootfs.0, rootfs.1, rootfs.2 (including their
childs), we have rootfs.0 set to 'active' so far and thus both rootfs.2
and rootfs.2 being detected as 'inactive'.

For this case, there is still no real definition of when or whether to select
rootfs.1 or rootfs.2.

Prior to this, the test checked for the selected slot being rootfs.1
which was valid for the concrete implementation, but mainly depending on
the order that was given by the GHashTable (iterator) implementation.

In recent glib version this changed resulting in rootfs.2 being selected
instead.

We adapt the test for checking correctly for both cases.

Supersedes #475

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 9, 2019

Codecov Report

Merging #476 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #476   +/-   ##
======================================
  Coverage    66.6%   66.6%           
======================================
  Files          22      22           
  Lines        6370    6370           
======================================
  Hits         4243    4243           
  Misses       2127    2127

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35c98ab...573ed27. Read the comment docs.

jluebbe
jluebbe previously approved these changes Sep 9, 2019
@ejoerns
Copy link
Copy Markdown
Member Author

ejoerns commented Sep 9, 2019

@elboulangero This should be of interest for you, too.

@ukleinek
Copy link
Copy Markdown
Member

I didn't look closely, but I wonder if we need something like ejoerns@9230105 for this commit to be correct.

@ejoerns
Copy link
Copy Markdown
Member Author

ejoerns commented Sep 10, 2019

@ukleinek it works without, but more accidentially than intended. And there is no guarantee that it will work in the future, too. Thus for this PR it makes sense to include the commit you mentioned to fully fix the known issues in this test.

@ukleinek
Copy link
Copy Markdown
Member

I would swap the order of the two commits, other than that it looks fine for me now

Copy link
Copy Markdown
Member

@ukleinek ukleinek left a comment

Choose a reason for hiding this comment

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

Typo in commit log: s/childs/children/

Copy link
Copy Markdown
Member

@ukleinek ukleinek left a comment

Choose a reason for hiding this comment

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

Typo in commit log: s/Someting/Something/

@ejoerns ejoerns force-pushed the topic/further-fixes-for-testing branch from bca21c4 to 604ba7c Compare September 11, 2019 07:48
@ejoerns
Copy link
Copy Markdown
Member Author

ejoerns commented Sep 11, 2019

@ukleinek I've fixed the typos and, while at it, I also changed the order of commits if that looks cleaner to you ;)

@ejoerns ejoerns requested a review from ukleinek September 11, 2019 07:50
jluebbe
jluebbe previously approved these changes Sep 11, 2019
This test lacked proper setting of 'bootslot' context property which is
required for a reliable test of determine_slot_states().

Up to now, the property was only set correctly by the previously
executed test. Something we should not rely on...

Signed-off-by: Enrico Joerns <ejo@pengutronix.de>
For the triple-redundancy rootfs.0, rootfs.1, rootfs.2 (including their
children), we have rootfs.0 set to 'active' so far and thus both rootfs.2
and rootfs.2 being detected as 'inactive'.

For this case, there is still no real definition of when or whether to select
rootfs.1 or rootfs.2.

Prior to this, the test checked for the selected slot being rootfs.1
which was valid for the concrete implementation, but mainly depending on
the order that was given by the GHashTable (iterator) implementation.

In recent glib version this changed resulting in rootfs.2 being selected
instead.

We adapt the test for checking correctly for both cases.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
@ejoerns
Copy link
Copy Markdown
Member Author

ejoerns commented Sep 11, 2019

@ukleinek really reliably fixed typos now.

Copy link
Copy Markdown
Member

@ukleinek ukleinek left a comment

Choose a reason for hiding this comment

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

LGTM now. Thanks for your effort to address these problems.

@ejoerns ejoerns merged commit aa435c2 into rauc:master Sep 11, 2019
@ejoerns ejoerns deleted the topic/further-fixes-for-testing branch September 11, 2019 12:59
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.

3 participants