Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(anta.cli): Add anta get tests #843

Open
wants to merge 42 commits into
base: main
Choose a base branch
from

Conversation

gmuloc
Copy link
Collaborator

@gmuloc gmuloc commented Sep 30, 2024

Description

Fixes #326

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have run pre-commit for code linting and typing (pre-commit run)
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (tox -e testenv)

@gmuloc gmuloc changed the title Feat(anta.cli): Add anta get tests feat(anta.cli): Add anta get tests Sep 30, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@gmuloc gmuloc added this to the v1.2.0 milestone Oct 21, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link

codspeed-hq bot commented Oct 29, 2024

CodSpeed Performance Report

Merging #843 will not alter performance

Comparing gmuloc:anta-get-tests (62de228) with main (91eac73)

Summary

✅ 8 untouched benchmarks

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@gmuloc gmuloc marked this pull request as ready for review November 14, 2024 14:26
@click.option("--test", help="Test name to retrieve the example for. If module is set, lookup only in given module.", required=False)
@click.option("--short", help="Print test names only and not the inputs", required=False, is_flag=True, default=False)
@click.command
def tests(module: str | None, test: str | None, *, short: bool) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also add anta get tests --count and display something like: Total tests available in ANTA: 154 or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can - you think it is useful or is it just for us? :p

Copy link
Contributor

Choose a reason for hiding this comment

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

Just for us :)

anta/cli/get/commands.py Outdated Show resolved Hide resolved
@click.command
def tests(module: str | None, test: str | None, *, short: bool) -> None:
"""Show all builtin ANTA tests with an example output retrieved from each test documentation."""
filterwarnings("ignore", message="Unknown section Expected Results")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment about why we can safely ignore that warning.

Comment on lines 146 to 149
if module:
explore_package(module, test_name=test, short=short)
else:
explore_package("anta.tests", test_name=test, short=short)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if module:
explore_package(module, test_name=test, short=short)
else:
explore_package("anta.tests", test_name=test, short=short)
module = module or "anta.tests"
explore_package(module, test_name=test, short=short)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, are we ok with empty strings? If we do anta get tests --module "" or anta get tests --test "" we basically get everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can be nice to the user and say "Hey your filter is empty" or something.

anta/cli/get/utils.py Outdated Show resolved Hide resolved
@@ -352,6 +352,7 @@ class VerifyAcctConsoleMethods(AntaTest):
```
"""

description = "Verifies the AAA accounting console method lists for different accounting types (system, exec, commands, dot1x)."
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this if it's the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm something with wrap I think - need to investigate again

Comment on lines +97 to +99
regex_patterns:
- "^enable password.*$"
- "bla bla"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the script still work if the indentation is not ok?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no

Copy link
Contributor

Choose a reason for hiding this comment

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

That's bad no? That means a dev can create a new test with wrong indentation, pre-commit will not add it, we will forget and the world is sad.

docs/scripts/generate_examples_tests.py Outdated Show resolved Hide resolved
prev = os.environ.get("TERM", "")
os.environ["TERM"] = "dumb"
# imported after TERM is set to act upon rich console.
from anta.cli.get.commands import tests # noqa: E402
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we ok with using click functions in scripts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm I would say yes - people developing should have click installed - thoughts?

Copy link
Contributor

@carl-baillargeon carl-baillargeon Nov 19, 2024

Choose a reason for hiding this comment

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

Maybe we should have a generic function called get_tests or something that we can use here and in the tests click function.

def print_tests_examples(qname: str, level: int, test_name: str | None, *, short: bool = False) -> None:
"""Print tests in qname if matching test_name (or all if test_name is None.

TODO: Allow to give a package argument to import_module.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unclear what that means?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what line?

Copy link
Contributor

Choose a reason for hiding this comment

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

The TODO. Is there a scenario where we could give a package argument for this use case? If not then TODO is not necessary.

Copy link

sonarcloud bot commented Nov 19, 2024

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.

Implement anta get tests to generate a test catalog example
2 participants