Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions tests/common/plugins/test_completeness/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@ CompletenessLevel marker enables testcases to be executed in different meaningfu
Each level is a representation of a scope of execution of a testcase. This document describes the usage of CompletenessLevel marker.
Identified meaningful levels (in increasing order) -

Debug
debug

Basic
basic

Confident
confident

Thorough
thorough

An unordered level `diagnose` is also supported. If `diagnose` level is specified and a test doesn’t support it, then it shall fall back to `basic` mode. This level doesn’t have any implication of above or below any existing level. Furthermore, this level is not the same as `debug` level, which is intended to take a sample test to debug test code. `diagnose` level is to be used to diagnose sonic feature/behavior.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

diagnose level is used to diagnose sonic feature/behavior.

Can you clarify this some more? I think it is still a little confusing what the difference is between this new level and the existing levels. Maybe an example use-case would help?

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.

Improved the description and described what the level is ideally meant for.


### To use CompletenessLevel:
- Mark the testcase with marker ```supported_completeness_level```. This marker is a list of all the completeness levels supported by a testcase.
- During Pytest execution, use command line option ```--completeness_level``` to specify the test completeness level.
- Automatic normalization between specified ```--completeness_level``` and defined ```supported_completeness_level``` will be performed and the test will be executed at the resultant normalized level of completeness.
- If module/session/testcase have different supported levels of completeness, the inner most level will supersede any defined level.
For eg., if the module and testcase have supported levels "debug, basic, thorough" and "confident" respectively, the resultant defined level for this testcase will be "confident".
- Within a testcase - Class method `CompletenessLevel.get_normalized_level()` can be called with the test's `request` object to get the normalized level.

### Different cases for CompletenessLevel

Expand All @@ -37,20 +40,20 @@ To handle any discrepancy between specified and defined completeness levels, nor
import pytest
from tests.common.plugins.test_completeness import CompletenessLevel

pytestmark = [pytest.mark.supported_completeness_level(CompletenessLevel.Debug, CompletenessLevel.Thorough)]
pytestmark = [pytest.mark.supported_completeness_level(CompletenessLevel.debug, CompletenessLevel.thorough)]

def test_test_completeness_default(request):
normalized_level = [mark.args for mark in request.node.iter_markers(name="supported_completeness_level")]
normalized_level = CompletenessLevel.get_normalized_level(request)
logger.info("Completeness level set to: {}".format(str(normalized_level)))

## Continue execution of the testecase until the completeness level specified.
# Debug - Do something - end the test if the specified level is Debug
# debug - Do something - end the test if the specified level is debug
...
...
# Basic - Do something more - extra tests/verifications - end the test now if the level is Basic
# basic - Do something more - extra tests/verifications - end the test now if the level is basic
...
...
# Thorough - Run entire test - if the set level is Thorough
# thorough - Run entire test - if the set level is thorough
```

### CompletenessLevel execution snippets
Expand Down
50 changes: 42 additions & 8 deletions tests/common/plugins/test_completeness/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,37 @@
import warnings

class CompletenessLevel(enum.IntEnum):
debug = 0 # Minimum execution
basic = 1
confident = 2
thorough = 3 # Maximum execution
diagnose = 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you did mention this is an unordered level. do you need to shift the other levels in that case? Wouldn't specifying some value for 'diagnose' level suffice?

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.

Any int value can be assigned to diagnose. It has to be int due to base type enum.IntEnum.
During normalization, there is a special handling for diagnose which does not consider ordering.

Copy link
Copy Markdown
Contributor Author

@vaibhavhd vaibhavhd Sep 1, 2020

Choose a reason for hiding this comment

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

I started with enum.IntEnum as the levels were assumed to follow a ordered priority. But this does not seem to be the case anymore. Removing base class inheritance will make this list look simple, but then the downside is we will have to implement methods to handle ordered levels, exists check, etc. These methods are presently used from enum.IntEnum

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to keep the change minimal, you can assign diagnose some int value since its treated differently

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.

Do you mean any int value other than 0?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes. some large value leaving enough space after max level in case we need to add more ordered levels in future

debug = 1 # Minimum execution
basic = 2
confident = 3
thorough = 4 # Maximum execution

@classmethod
def get_normalized_level(cls, request):
"""Takes a request instance and returns normalized completeness level in string format.
For example, if a testcase supports "CompletenessLevel.basic, CompletenessLevel.thorough", and specified level
during test execution is "confident", this method will make use of normalization logic during
pytest_runtest_setup and returns the normalized level as "basic" (str type of CompletenessLevel.basic)
"""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a few suggestions here:

  1. Separate the summary line and the example
  2. Generalize the description a little bit (i.e. describe what is happening rather than how it is happening)
  3. Explicitly call out the return type

One possible interpretation:

Suggested change
"""Takes a request instance and returns normalized completeness level in string format.
For example, if a testcase supports "CompletenessLevel.basic, CompletenessLevel.thorough", and specified level
during test execution is "confident", this method will make use of normalization logic during
pytest_runtest_setup and returns the normalized level as "basic" (str type of CompletenessLevel.basic)
"""
"""Get the normalized completeness level for a given test instance.
For example, if a testcase supports "CompletenessLevel.basic, CompletenessLevel.thorough", and the specified level
during test execution is "confident", then this method will normalize the level to "basic".
Returns:
CompletenessLevel as a string
"""

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.

Incorporated the suggestion. Thanks!

all_supported_levels = [mark.args for mark in request.node.iter_markers(name="supported_completeness_level")]
logging.info("All supported completeness levels of the test: {}".format(str(all_supported_levels)))
normalized_level = all_supported_levels[0][0]
normalized_level_name = CompletenessLevel.get_level_name(normalized_level)
logging.info("Normalized completeness level set to: {}".format(normalized_level_name))
return normalized_level_name

@classmethod
def get_level_name(cls, level):
"""Converts a type CompletenessLevel to type str.
For example, if input is CompletenessLevel.basic, this method will return "basic"
Arguments:
level - An enum value of type CompletenessLevel
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just some minor formatting changes:

Suggested change
"""Converts a type CompletenessLevel to type str.
For example, if input is CompletenessLevel.basic, this method will return "basic"
Arguments:
level - An enum value of type CompletenessLevel
"""Converts a type CompletenessLevel to type str.
For example, if input is CompletenessLevel.basic, this method will return "basic."
Arguments:
level - An enum value of type CompletenessLevel

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.

Incorporated the suggestion. Thanks!

"""
if type(level) is not CompletenessLevel:
logging.error("Invalid completeness type. Expected: {}. Format {}".format(str(CompletenessLevel), type(level)))
level_name = level.name.lower()
return level_name

def set_default(specified_level):
if not specified_level: # Case 1
Expand All @@ -17,16 +44,22 @@ def set_default(specified_level):
specified_level = specified_level.lower()
if specified_level not in CompletenessLevel._member_names_:
specified_level = CompletenessLevel.basic
warnings.warn("Unidentified completeness level specified. Specified: {}. Allowed: {}".format(str(CompletenessLevel(specified_level)), \
str(CompletenessLevel._member_names_)))
warnings.warn("Unidentified completeness level specified. Specified: {}. Allowed: {}"\
.format(str(CompletenessLevel(specified_level)), str(CompletenessLevel._member_names_)))
logging.info("Unidentified completeness level specified. Setting to default level: {}".format(CompletenessLevel.basic))
else:
specified_level = CompletenessLevel[specified_level]

return specified_level

def normalize_levels(specified_level, defined_levels):
logging.info("Setting test completeness level. Specified: {}. Defined: {}".format(str(CompletenessLevel(specified_level)), str(defined_levels)))
logging.info("Setting test completeness level. Specified: {}. Defined: {}".\
format(str(CompletenessLevel(specified_level)), str(defined_levels)))

if specified_level not in defined_levels:
# if specified_level is diagnose and the testcase does not support it, default level is basic.
if specified_level == CompletenessLevel.diagnose:
specified_level = CompletenessLevel.basic

if specified_level not in defined_levels:
if specified_level > max(defined_levels): # Case 3.1
Expand All @@ -40,7 +73,8 @@ def normalize_levels(specified_level, defined_levels):
if level <= specified_level and lesser_defined_level_dist > (specified_level - level):
completeness_level = level
lesser_defined_level_dist = lesser_defined_level_dist - level
logging.info("Specified level ({}) not found in defined levels. Setting level to {}".format(str(CompletenessLevel(specified_level)), str(completeness_level)))
logging.info("Specified level ({}) not found in defined levels. Setting level to {}".\
format(str(CompletenessLevel(specified_level)), str(completeness_level)))
else: # Case 4
completeness_level = specified_level
logging.info("Setting the completeness level to {}".format(str(CompletenessLevel(completeness_level))))
Expand Down
17 changes: 9 additions & 8 deletions tests/platform_tests/test_link_flap.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
]

class TestLinkFlap:
def __init__(self, request):
self.completeness_level = CompletenessLevel.get_normalized_level(request)

def __get_dut_if_status(self, dut, ifname=None):
if not ifname:
status = dut.show_interface(command='status')['ansible_facts']['int_status']
Expand Down Expand Up @@ -47,7 +50,7 @@ def __toggle_one_link(self, dut, dut_port, fanout, fanout_port):
self.ports_shutdown_by_test.discard((fanout, fanout_port))


def __build_test_candidates(self, dut, fanouthosts, completeness_level):
def __build_test_candidates(self, dut, fanouthosts):
status = self.__get_dut_if_status(dut)
candidates = []

Expand All @@ -60,17 +63,17 @@ def __build_test_candidates(self, dut, fanouthosts, completeness_level):
logging.info("Skipping port {} that is admin down".format(dut_port))
else:
candidates.append((dut_port, fanout, fanout_port))
if CompletenessLevel.debug in completeness_level:
if self.completeness_level == 'debug':
# Run the test for one port only - to just test if the test works fine
return candidates

return candidates


def run_link_flap_test(self, dut, fanouthosts, completeness_level):
def run_link_flap_test(self, dut, fanouthosts):
self.ports_shutdown_by_test = set()

candidates = self.__build_test_candidates(dut, fanouthosts, completeness_level)
candidates = self.__build_test_candidates(dut, fanouthosts)
if not candidates:
pytest.skip("Didn't find any port that is admin up and present in the connection graph")

Expand All @@ -85,7 +88,5 @@ def run_link_flap_test(self, dut, fanouthosts, completeness_level):

@pytest.mark.platform('physical')
def test_link_flap(request, duthost, fanouthosts):
normalized_level = [mark.args for mark in request.node.iter_markers(name="supported_completeness_level")][0]
logging.info("Completeness level set: {}".format(str(normalized_level)))
tlf = TestLinkFlap()
tlf.run_link_flap_test(duthost, fanouthosts, normalized_level)
tlf = TestLinkFlap(request)
tlf.run_link_flap_test(duthost, fanouthosts)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing newline at the end. (I know you didn't add this but since we're here. 😄)

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.

Added.