Skip to content

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

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.

Sign up for GitHub

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

Closed
martinaldrin wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Current
Fixed: GITHUB-2643: assertEquals(Set,Set) now ignores ordering as it did before. (Elis Edlund)
Fixed: GITHUB-2653: Assert methods requires casting since TestNg 7.0 for mixed boxed and unboxed primitives in assertEquals.
New: GITHUB-2663 Add support for Priority in Configuration Methods. (Martin Aldrin)
Fixed: GITHUB-2653: Assert methods requires casting since TestNg 7.0 for mixed boxed and unboxed primitives in assertEquals. (Martin Aldrin)
Fixed: GITHUB-2229: Restore @BeforeGroups and @AfterGroups Annotations functionality (Krishnan Mahadevan)
Fixed: GITHUB-2563: Skip test if its data provider provides no data (Krishnan Mahadevan)
Fixed: GITHUB-2535: TestResult.getEndMillis() returns 0 for skipped configuration - after upgrading testng to 7.0 + (Krishnan Mahadevan)
Expand Down
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/AfterClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/AfterGroups.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/AfterMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/AfterSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@
* @return the valude (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/AfterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/BeforeClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/BeforeGroups.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/BeforeMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/BeforeSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/BeforeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,11 @@
* @return the value (default 0)
*/
long timeOut() default 0;

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int priority() default 0;
}
7 changes: 7 additions & 0 deletions testng-core-api/src/main/java/org/testng/annotations/ITestOrConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,11 @@ public interface ITestOrConfiguration extends IParameterizable {
String getDescription();

void setDescription(String description);

/**
* The scheduling priority. Lower priorities will be scheduled first.
*
* @return the value (default 0)
*/
int getPriority();
}
4 changes: 2 additions & 2 deletions testng-core/src/main/java/org/testng/internal/ClonedMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ public int getPriority() {

@Override
public void setPriority(int priority) {
// ignored
m_method.setPriority(priority);
}

@Override
Expand All @@ -337,7 +337,7 @@ public int getInterceptedPriority() {

@Override
public void setInterceptedPriority(int priority) {
// ignored
m_method.setInterceptedPriority(priority);
}

@Override
Expand Down
2 changes: 2 additions & 0 deletions testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ private void init() {
m_inheritGroupsFromTestClass = annotation.getInheritGroups();
setEnabled(annotation.getEnabled());
setDescription(annotation.getDescription());
setPriority(annotation.getPriority());
}

if (annotation != null && annotation.isFakeConfiguration()) {
Expand Down Expand Up @@ -503,6 +504,7 @@ public ConfigurationMethod clone() {
clone.setDescription(getDescription());
clone.setEnabled(getEnabled());
clone.setParameterInvocationCount(getParameterInvocationCount());
clone.setPriority(getPriority());
clone.m_inheritGroupsFromTestClass = inheritGroupsFromTestClass();

return clone;
Expand Down
20 changes: 7 additions & 13 deletions testng-core/src/main/java/org/testng/internal/MethodHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.testng.internal.collections.Pair;
import org.testng.internal.invokers.IInvocationStatus;
import org.testng.util.TimeUtils;
import org.testng.xml.XmlTest;

/** Collection of helper methods to help sort and arrange methods. */
public class MethodHelper {
Expand Down Expand Up @@ -259,11 +258,7 @@ private static Graph<ITestNGMethod> topologicalSort(

Map<Object, List<ITestNGMethod>> testInstances = sortMethodsByInstance(methods);

XmlTest xmlTest = null;
for (ITestNGMethod m : methods) {
if (xmlTest == null) {
xmlTest = m.getXmlTest();
}
result.addNode(m);

List<ITestNGMethod> predecessors = Lists.newArrayList();
Expand All @@ -289,14 +284,13 @@ private static Graph<ITestNGMethod> topologicalSort(
}
predecessors.addAll(Arrays.asList(methodsNamed));
}
if (XmlTest.isGroupBasedExecution(xmlTest)) {
String[] groupsDependedUpon = m.getGroupsDependedUpon();
if (groupsDependedUpon.length > 0) {
for (String group : groupsDependedUpon) {
ITestNGMethod[] methodsThatBelongToGroup =
MethodGroupsHelper.findMethodsThatBelongToGroup(m, methods, group);
predecessors.addAll(Arrays.asList(methodsThatBelongToGroup));
}
// Should not be part of this commit. just added until GITHUB-2664 is solved.
String[] groupsDependedUpon = m.getGroupsDependedUpon();
if (groupsDependedUpon.length > 0) {
for (String group : groupsDependedUpon) {
ITestNGMethod[] methodsThatBelongToGroup =
MethodGroupsHelper.findMethodsThatBelongToGroup(m, methods, group);
predecessors.addAll(Arrays.asList(methodsThatBelongToGroup));
}
}

Expand Down
12 changes: 5 additions & 7 deletions testng-core/src/main/java/org/testng/internal/MethodInheritance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.testng.ITestNGMethod;
import org.testng.collections.Lists;
import org.testng.collections.Maps;
import org.testng.xml.XmlTest;

public class MethodInheritance {

Expand Down Expand Up @@ -135,14 +134,13 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean before)
ITestNGMethod m1 = l.get(i);
for (int j = i + 1; j < l.size(); j++) {
ITestNGMethod m2 = l.get(j);
boolean groupMode = XmlTest.isGroupBasedExecution(m2.getXmlTest());
if (groupMode) {
// Do not resort to adding implicit depends-on if there are groups
continue;
}
if (!equalsEffectiveClass(m1, m2) && !dependencyExists(m1, m2, methods)) {
Utils.log("MethodInheritance", 4, m2 + " DEPENDS ON " + m1);
m2.addMethodDependedUpon(MethodHelper.calculateMethodCanonicalName(m1));
if (m1.getPriority() == 0
&& m2.getPriority() == 0
&& m1.getPriority() >= m2.getPriority()) {
Copy link
Copy Markdown
Contributor

@Elisedlund-ericsson Elisedlund-ericsson Nov 1, 2021

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

@martinaldrin martinaldrin Nov 1, 2021

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

@martinaldrin martinaldrin Nov 1, 2021

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

@cbeust cbeust Nov 1, 2021

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

@martinaldrin martinaldrin Nov 1, 2021

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.

m2.setPriority(m2.getPriority() + 1);
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions testng-core/src/main/java/org/testng/internal/annotations/AnnotationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ private static void finishInitialize(
result.setGroups(bs.getGroups());
result.setInheritGroups(bs.getInheritGroups());
result.setTimeOut(bs.getTimeOut());
result.setPriority(bs.getPriority());
}

public static List<Class<? extends IAnnotation>> getAllAnnotations() {
Expand Down
12 changes: 12 additions & 0 deletions testng-core/src/main/java/org/testng/internal/annotations/JDK15TagFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IAfterSuite.class) {
AfterSuite bs = (AfterSuite) a;
Expand Down Expand Up @@ -136,6 +137,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IBeforeTest.class) {
BeforeTest bs = (BeforeTest) a;
Expand Down Expand Up @@ -163,6 +165,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IAfterTest.class) {
AfterTest bs = (AfterTest) a;
Expand Down Expand Up @@ -190,6 +193,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IBeforeGroups.class) {
BeforeGroups bs = (BeforeGroups) a;
Expand Down Expand Up @@ -218,6 +222,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IAfterGroups.class) {
AfterGroups bs = (AfterGroups) a;
Expand Down Expand Up @@ -246,6 +251,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IBeforeClass.class) {
BeforeClass bs = (BeforeClass) a;
Expand Down Expand Up @@ -273,6 +279,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IAfterClass.class) {
AfterClass bs = (AfterClass) a;
Expand Down Expand Up @@ -300,6 +307,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
false,
bs.timeOut(),
bs.priority(),
new String[0]);
} else if (annotationClass == IBeforeMethod.class) {
BeforeMethod bs = (BeforeMethod) a;
Expand Down Expand Up @@ -327,6 +335,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
bs.firstTimeOnly(),
false,
bs.timeOut(),
bs.priority(),
bs.onlyForGroups());
} else if (annotationClass == IAfterMethod.class) {
AfterMethod bs = (AfterMethod) a;
Expand Down Expand Up @@ -354,6 +363,7 @@ private IAnnotation maybeCreateNewConfigurationTag(Annotation a, Class<?> annota
false,
bs.lastTimeOnly(),
bs.timeOut(),
bs.priority(),
bs.onlyForGroups());
}

Expand Down Expand Up @@ -383,6 +393,7 @@ private IAnnotation createConfigurationTag(
boolean firstTimeOnly,
boolean lastTimeOnly,
long timeOut,
int priority,
String[] groupFilters) {
ConfigurationAnnotation result = new ConfigurationAnnotation();
result.setIsBeforeGroups(isBeforeGroups);
Expand Down Expand Up @@ -410,6 +421,7 @@ private IAnnotation createConfigurationTag(
result.setFirstTimeOnly(firstTimeOnly);
result.setLastTimeOnly(lastTimeOnly);
result.setTimeOut(timeOut);
result.setPriority(priority);

return result;
}
Expand Down
1 change: 1 addition & 0 deletions testng-core/src/main/java/org/testng/internal/annotations/TestOrConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public void setDescription(String description) {
m_description = description;
}

@Override
public int getPriority() {
return m_priority;
}
Expand Down
Loading