enapps-enorman commented on code in PR #25:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/25#discussion_r1294029797
##########
src/test/java/org/apache/sling/jcr/contentloader/it/ContentloaderTestSupport.java:
##########
@@ -39,7 +40,6 @@
import java.io.InputStream;
import java.io.StringWriter;
import java.text.SimpleDateFormat;
-import java.time.Duration;
Review Comment:
Add this back when switching to awaitility 4.x
##########
src/test/java/org/apache/sling/jcr/contentloader/it/ContentloaderTestSupport.java:
##########
@@ -18,6 +18,7 @@
*/
package org.apache.sling.jcr.contentloader.it;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
Review Comment:
Remove this when switching to awaitility 4.x
##########
pom.xml:
##########
@@ -383,10 +383,11 @@
<version>${org.ops4j.pax.exam.version}</version>
<scope>test</scope>
</dependency>
+ <!-- can't go beyond org.awaitility 3.1.6 because PAX exam 4.0.0 is
hard coded to use this-->
Review Comment:
I'm not really sure why o.a.sling.testing.paxexam has changed the awaitility
version to the older 3.x version instead of the 4.x version that it used to be.
I think we should try to not go backwards to the 3.x version of awaitility
if we can avoid it. I think the syntax of java.time.Duration is more readable
than the older syntax of org.awaitility.Duration constructors.
I think that a workaround could be accomplished by providing a custom
ContentloaderTestSupport#awaitility method to provide a different
implementation that pulls in the latest awaitility library instead of the old
one.
Something like this new method in ContentloaderTestSupport should work?
```
/**
* Replacement for {@link SlingOptions#awaitility()} to utilize a newer
version of awaitility
*
* NOTE: may remove this at a later date and go back to {@link
SlingOptions#awaitility()} whenever
* {@link SlingVersionResolver} provides these versions or later
*/
public static ModifiableCompositeOption awaitility() {
return composite(
mavenBundle().groupId("org.awaitility").artifactId("awaitility").versionAsInProject(),
mavenBundle().groupId("org.hamcrest").artifactId("hamcrest").version(SlingOptions.versionResolver)
);
}
```
##########
pom.xml:
##########
@@ -383,10 +383,11 @@
<version>${org.ops4j.pax.exam.version}</version>
<scope>test</scope>
</dependency>
+ <!-- can't go beyond org.awaitility 3.1.6 because PAX exam 4.0.0 is
hard coded to use this-->
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
- <version>4.2.0</version>
Review Comment:
I'd like this to go back to the 4.2.0 version if possible.
##########
src/test/java/org/apache/sling/jcr/contentloader/it/ContentloaderTestSupport.java:
##########
@@ -59,6 +59,7 @@
import org.apache.sling.testing.paxexam.SlingOptions;
import org.apache.sling.testing.paxexam.TestSupport;
import org.awaitility.Awaitility;
+import org.awaitility.Duration;
Review Comment:
Remove this when switching to awaitility 4.x
##########
src/test/java/org/apache/sling/jcr/contentloader/it/ContentloaderTestSupport.java:
##########
@@ -18,6 +18,7 @@
*/
package org.apache.sling.jcr.contentloader.it;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.apache.felix.hc.api.FormattingResultLog.msHumanReadable;
import static org.apache.sling.testing.paxexam.SlingOptions.awaitility;
Review Comment:
Remove this if using the custom ContentloaderTestSupport#awaitility method
instead.
##########
src/test/java/org/apache/sling/jcr/contentloader/it/ContentloaderTestSupport.java:
##########
@@ -196,8 +190,8 @@ protected void waitForContentLoaded() throws Exception {
*/
protected void waitForContentLoaded(long timeoutMsec, long
nextIterationDelay) throws Exception {
Awaitility.await("waitForContentLoaded")
- .atMost(Duration.ofMillis(timeoutMsec))
Review Comment:
If switching back to awaitility 4.x then these two changed lines should be
reverted back to how it was before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
