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

Hardcoded nrunner removal in TestSuite #5955

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

richtja
Copy link
Contributor

@richtja richtja commented Jun 12, 2024

This is a removal of left over of legacy code. When avocado supported legacy and nrunner runners we also distinguished between resolver resolution and test load. When the legacy runner has been removed, we don't have this issue anymore, because resolvers are only supported implementation of test resolution. Unfortunately, we kept validation of supported runners during the test reference resolution with hardcoded nrunner. This is wrong because nrunner is a SuiteRunner plugin, and it can be replaced by any installed SuiteRunner.

Reference: #5954

This is a removal of left over of legacy code. When avocado supported
`legacy` and `nrunner` runners we also distinguished between resolver
resolution and test load. When the `legacy` runner has been removed, we
don't have this issue anymore, because resolvers are only supported
implementation of test resolution. Unfortunately, we kept validation of
supported runners during the test reference resolution with hardcoded
`nrunner`. This is wrong because `nrunner` is a `SuiteRunner` plugin,
and it can be replaced by any installed `SuiteRunner`.

Reference: avocado-framework#5954
Signed-off-by: Jan Richter <[email protected]>
@richtja richtja added the bug label Jun 12, 2024
@richtja richtja added this to the #106 - Codename TBD milestone Jun 12, 2024
@richtja richtja requested review from clebergnu and harvey0100 June 12, 2024 12:57
@richtja richtja self-assigned this Jun 12, 2024
@richtja
Copy link
Contributor Author

richtja commented Jun 12, 2024

Hi @pevogam, when you will have time, can you please check if this fixes your issue in #5954, thank you.

@pevogam
Copy link
Contributor

pevogam commented Jun 12, 2024

Hi @pevogam, when you will have time, can you please check if this fixes your issue in #5954, thank you.

Hi @richtja, but I don't understand - is this run.suite_runner config option now deprecated? Because the diff in the fix simply seems to remove it.

@richtja
Copy link
Contributor Author

richtja commented Jun 12, 2024

Hi @pevogam, when you will have time, can you please check if this fixes your issue in #5954, thank you.

Hi @richtja, but I don't understand - is this run.suite_runner config option now deprecated? Because the diff in the fix simply seems to remove it.

Hi @pevogam, no it is not deprecated, the suite_runner is selected at different place. This code can be removed, because we had it only for distinguishing between legacy loader and nrunner resolver. Actually, this part of the code has nothing to do with the suite_runner itself, and therefore it can be removed. For more information about this, you can look into last LTS version with the legacy runner.

Copy link
Contributor

@pevogam pevogam left a comment

Choose a reason for hiding this comment

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

I see, in this case this change is entirely welcome. Our plugin sets exactly that config option and it works well with the exception of exactly the code that was dropped. LGTM

Copy link
Contributor

@clebergnu clebergnu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@clebergnu clebergnu merged commit 47976d4 into avocado-framework:master Jun 13, 2024
58 checks passed
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Oct 8, 2024
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
@richtja richtja deleted the scheduler_fix branch October 9, 2024 14:33
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Oct 17, 2024
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Jan 27, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Jan 29, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Jan 29, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Jan 31, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Jan 31, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Feb 10, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Feb 10, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Feb 12, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Feb 12, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
pevogam added a commit to intra2net/avocado-i2n that referenced this pull request Feb 15, 2025
This drops our previous workaround which was reported as needed in

avocado-framework/avocado#5954

and consequently resolved with the merge of the pull request

avocado-framework/avocado#5955
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Extra flexibility for the default test suite class for custom schedulers
4 participants