Skip to content

Restore Ordering for DependsOnGroups by using Priority on Configuration Methods alternative 1#2667

Closed
martinaldrin wants to merge 1 commit intotestng-team:masterfrom
martinaldrin:priority-with-fix
Closed

Restore Ordering for DependsOnGroups by using Priority on Configuration Methods alternative 1#2667
martinaldrin wants to merge 1 commit intotestng-team:masterfrom
martinaldrin:priority-with-fix

Conversation

@martinaldrin
Copy link
Copy Markdown

Restore Ordering for DependsOnGroups by using Priority on Configuration Methods

Fixes # .

Did you remember to?

  • Add test case(s)
  • Update CHANGES.txt

We encourage pull requests that:

  • Add new features to TestNG (or)
  • Fix bugs in TestNG

If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.

Restore Ordering for DependsOnGroups.
m2.addMethodDependedUpon(MethodHelper.calculateMethodCanonicalName(m1));
if (m1.getPriority() == 0
&& m2.getPriority() == 0
&& m1.getPriority() >= m2.getPriority()) {
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 dont see how m1.getPriority() >= m2.getPriority() can happen if it was already comfirmed that both ==0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, it can not happen. that was my first implementation, then I change to only check if both are 0. I don't want to risk modify this in case the users have configured priority.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I guess the best would be to have a separate priority implementation for class hierarchy to avoid interfere with the user configuration.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, the >= test will always be true, so it can be removed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @cbeust
Yes, an alternative solution can be found in #2668. When I now which solution is preferred I will cancel the others and rework the PR. Maybe it is to risky to reuse the priority and therefore I created a separate PR with a separate priority.

@martinaldrin martinaldrin changed the title Restore Ordering for DependsOnGroups by using Priority on Configuration Methods Restore Ordering for DependsOnGroups by using Priority on Configuration Methods alternative 1 Nov 2, 2021
@martinaldrin
Copy link
Copy Markdown
Author

I close this PR since it was just a Proof of Concept, will continue in #2666

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.

3 participants