Use dedicated DISABLED() function to disable tests in Python test framework

[grom72]

This change is 

[codecov]

Codecov Report

Merging #5792 (449cf30) into master (91711db) will decrease coverage by 2.25%.
The diff coverage is n/a.

❗ Current head 449cf30 differs from pull request most recent head 8b9b4fb. Consider uploading reports for the commit 8b9b4fb to get more accurate results

@@            Coverage Diff             @@
##           master    #5792      +/-   ##
==========================================
- Coverage   70.98%   68.74%   -2.25%     
==========================================
  Files         131      131              
  Lines       19175    19677     +502     
  Branches     3193     3259      +66     
==========================================
- Hits        13612    13527      -85     
- Misses       5563     6150     +587     

[janekmi]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @grom72 and @osalyk)

---

src/test/unittest/basetest.py line 149 at r2 (raw file):

    """
    enabled = True
    disabled = False

Having both these variables next to each other worries me a bit.
Why do we need both of them?
I guess for some reason disabled = True is not an equivalent of enabled = False?

---

src/test/unittest/utils.py line 30 at r2 (raw file):

def DISABLED(**kwargs):
    """
    DISABLE given test.

Suggestion:

    DISABLE a given test.

[grom72]

Reviewable status: 1 of 5 files reviewed, 2 unresolved discussions (waiting on @janekmi and @osalyk)

---

src/test/unittest/basetest.py line 149 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Having both these variables next to each other worries me a bit.
Why do we need both of them?
I guess for some reason disabled = True is not an equivalent of enabled = False?

enabled = False is used for silently disabling tests that:

• can not be run because of unsupported architecture
  

  pmdk/src/test/unittest/utils.py

  Line 22 in f0815d4

  tc.enabled = False

• can not be run because of unsupported(uninstalled) tool (e.g. gdb)
  

  pmdk/src/test/unittest/requirements.py

  Line 159 in f0815d4

  def require_command(command):

In both cases, we do not want to see any messages.
A different situation is in the case of disabled = True, where we explicitly disable the test and want to follow this fact in the log.

---

src/test/unittest/utils.py line 30 at r2 (raw file):

def DISABLED(**kwargs):
    """
    DISABLE given test.

Done.

[osalyk]

Reviewed 1 of 4 files at r1, 3 of 4 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @janekmi)

[janekmi]

Reviewed 3 of 4 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

---

src/test/unittest/basetest.py line 149 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

enabled = False is used for silently disabling tests that:

• can not be run because of unsupported architecture
  

  pmdk/src/test/unittest/utils.py

  Line 22 in f0815d4

  tc.enabled = False

• can not be run because of unsupported(uninstalled) tool (e.g. gdb)
  

  pmdk/src/test/unittest/requirements.py

  Line 159 in f0815d4

  def require_command(command):

In both cases, we do not want to see any messages.
A different situation is in the case of disabled = True, where we explicitly disable the test and want to follow this fact in the log.

Got it. But TBH as a casual user of this framework I would prefer to see a message when a test I requested to be run skips no matter the reason. Otherwise, as a user, I am left with no clue whatsoever. Just a thought.

---

src/test/unittest/basetest.py line 148 at r4 (raw file):

        force_disabling (bool): True if test is explicitle excluded from
            execution e.g. in case of permanent failing test.
            Defaults to False.

Suggestion:

        force_disabling (bool): True if the test is explicitly excluded from
            execution e.g. in case the test fails permanently.
            Defaults to False.

---

src/test/unittest/basetest.py line 152 at r4 (raw file):

    """
    enabled = True
    force_disabling = False

I would prefer to call it force_disabled. Especially the rest of the code follows the "disabled" scheme.

[grom72]

Reviewed 2 of 4 files at r5, 1 of 1 files at r6.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @janekmi and @osalyk)

---

src/test/unittest/basetest.py line 149 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Got it. But TBH as a casual user of this framework I would prefer to see a message when a test I requested to be run skips no matter the reason. Otherwise, as a user, I am left with no clue whatsoever. Just a thought.

I think that in the case of require_command, we can use SKIP-base solution or even explicitly fail as we do with Valgrind
But what can we do with unsupported architecture?
Shall we skip it, or shall we remove it at all?
I propose to move this discussion to a separate PR.

---

src/test/unittest/basetest.py line 148 at r4 (raw file):

        force_disabling (bool): True if test is explicitle excluded from
            execution e.g. in case of permanent failing test.
            Defaults to False.

Done

---

src/test/unittest/basetest.py line 152 at r4 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I would prefer to call it force_disabled. Especially the rest of the code follows the "disabled" scheme.

Done.

[grom72]

Reviewed 1 of 4 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @janekmi)

[grom72]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)

[janekmi]

Reviewed 3 of 4 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72)