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

nrunner: reorganization of unittests #5966

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

clebergnu
Copy link
Contributor

There are some "blind spots" in the runnable code, such as when dealing with serialization from JSON files and Runnable's configuration.

To allow for a clearer view of the current coverage of Runnable's features, let's reorganize their tests in a file of its own, and based on the usage and where the data is coming from or going to.

@clebergnu clebergnu force-pushed the unittest_nrunner_reorg branch 3 times, most recently from f165220 to 597a771 Compare June 24, 2024 13:07
@clebergnu clebergnu marked this pull request as ready for review June 24, 2024 15:56
@clebergnu clebergnu requested review from richtja and harvey0100 June 24, 2024 15:56
Copy link
Contributor

@richtja richtja left a comment

Choose a reason for hiding this comment

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

Hi @clebergnu, the overall changes LGTM. I have just one proposal about removing some tests, but we can merge it without it. And one proposal to commit format which would help me to finish the review.

selftests/unit/nrunner.py Show resolved Hide resolved
selftests/check.py Outdated Show resolved Hide resolved
@clebergnu clebergnu added this to the 107 - Codename TBD milestone Jul 2, 2024
@clebergnu clebergnu force-pushed the unittest_nrunner_reorg branch 2 times, most recently from a468d77 to 68f5f5e Compare July 10, 2024 10:52
@clebergnu clebergnu requested a review from richtja July 10, 2024 11:12
Copy link
Contributor

@richtja richtja left a comment

Choose a reason for hiding this comment

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

Hi @clebergnu, thank you for your updates. I have one more comment to the runner_tap.py path. Please let me know what do you think.

selftests/unit/runner_tap.py Outdated Show resolved Hide resolved
The temp directory is being used simply to hold the content of
runnable recipes (JSON files) which are created by means of
Runnable.write_json().  But, Runnable.write_json() itself is
utter uninteresting, being simply a convenience around
Runnable.get_json().

Signed-off-by: Cleber Rosa <[email protected]>
This avoids some codepaths that can mock the loading of the JSON
schema, instead of the opening of the recipe file.

Signed-off-by: Cleber Rosa <[email protected]>
There are some "blind spots" in the runnable code, such as when
dealing with serialization from JSON files and Runnable's
configuration.

To allow for a clearer view of the current coverage of Runnable's
features, let's reorganize their tests in a file of its own, and based
on the usage and where the data is coming from or going to.

Signed-off-by: Cleber Rosa <[email protected]>
@clebergnu clebergnu force-pushed the unittest_nrunner_reorg branch from 68f5f5e to c97ca92 Compare July 12, 2024 08:15
@clebergnu clebergnu requested a review from richtja July 12, 2024 08:16
Copy link
Contributor

@richtja richtja left a comment

Choose a reason for hiding this comment

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

After the changes it LGTM, thanks @clebergnu.

@richtja richtja merged commit 438b38f into avocado-framework:master Jul 12, 2024
56 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants