Add unit test basics for URLFrontier. #984

FelixEngl · 2022-07-05T12:01:20Z

Adds basics for Unit-Testing the URLFrontier and a test for ACKing of the URLItems. (The classes Constants.java and ChannelManager.java are from #982)

Signed-off-by: Felix Engl felix.engl@uni-bamberg.de

Thanks for contributing to StormCrawler, your efforts are appreciated!

Developer Certificate of Origin

By contributing to StormCrawler, you accept and agree to the following terms and conditions (the Developer Certificate of Origin) for your present and future contributions submitted to StormCrawler.
Please refer to the Developer Certificate of Origin section in CONTRIBUTING.md for details.

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Before opening a PR, please check that:

You've squashed your commits into a single one
You've described what the PR does or at least point to a related issue
You've signed-ff your commits with 'git commit -s'
The code is properly formatted with 'mvn git-code-format:format-code -Dgcf.globPattern=**/*'

Thanks!

Signed-off-by: Felix Engl <felix.engl@uni-bamberg.de>

jnioche

Could you please base this PR against the latest main branch now that I merged your other PR? It will be a lot cleaner and easier to review.

...nal/urlfrontier/src/test/java/com/digitalpebble/stormcrawler/urlfrontier/StatusBoldTest.java

FelixEngl · 2022-07-06T08:16:24Z

I realized that we get a suppressed error when running the test with the code in the commit but the same test works just fine with the changes in #980

10:13:06.043 [Time-limited test] TRACE c.d.s.u.StatusUpdaterBolt - Added to waitAck https://www.url.net/something with ID https://www.url.net/something total 1 - sent to localhost:52023
10:13:06.991 [grpc-default-executor-1] DEBUG c.d.s.u.StatusUpdaterBolt - Acked 1 tuple(s) for ID https://www.url.net/something
10:13:07.030 [grpc-default-executor-1] ERROR c.d.s.u.StatusUpdaterBolt - Error received
io.grpc.StatusRuntimeException: UNAVAILABLE: Channel shutdownNow invoked
	at io.grpc.Status.asRuntimeException(Status.java:535) ~[grpc-api-1.44.0.jar:1.44.0]
	at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:479) ~[grpc-stub-1.44.0.jar:1.44.0]
	at io.grpc.internal.DelayedClientCall$DelayedListener$3.run(DelayedClientCall.java:463) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.DelayedClientCall$DelayedListener.delayOrExecute(DelayedClientCall.java:427) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.DelayedClientCall$DelayedListener.onClose(DelayedClientCall.java:460) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.ClientCallImpl.closeObserver(ClientCallImpl.java:562) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.ClientCallImpl.access$300(ClientCallImpl.java:70) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInternal(ClientCallImpl.java:743) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInContext(ClientCallImpl.java:722) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37) ~[grpc-core-1.44.0.jar:1.44.0]
	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:133) ~[grpc-core-1.44.0.jar:1.44.0]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) ~[?:?]
	at java.lang.Thread.run(Thread.java:829) ~[?:?]

Process finished with exit code 0

I think this is related to starvation / async problems.

Do you have an idea how we could test this properly?

Signed-off-by: Felix Engl <felix.engl@uni-bamberg.de>

…RLFrontier_AddFirstTest

...frontier/src/test/java/com/digitalpebble/stormcrawler/urlfrontier/StatusUpdaterBoltTest.java

jnioche · 2022-07-06T10:04:44Z

...frontier/src/test/java/com/digitalpebble/stormcrawler/urlfrontier/StatusUpdaterBoltTest.java

+
+    @Before
+    public void before() {
+


I'd rather not have to change a hardcoded version number in a class when it won't be used most of the time. Let's do this instead - it works fine.

String image = "crawlercommons/url-frontier"; String version = System.getProperty("urlfrontier-version"); if (version != null) image += ":" + version; urlFrontierContainer = new URLFrontierContainer(image); urlFrontierContainer.start();

jnioche · 2022-07-06T10:11:21Z

...frontier/src/test/java/com/digitalpebble/stormcrawler/urlfrontier/StatusUpdaterBoltTest.java

+    public void canAckASimpleTuple()
+            throws ExecutionException, InterruptedException, TimeoutException {
+
+        Configurator.setLevel(StatusUpdaterBolt.class, Level.ALL);


log4j - please remove alongside any other use of it and rely on slf4j instead

slf4j does not give the option to set a global logging level like log4j.
Should I drop it?

what about something like this (not tested)

LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory(); loggerContext.getLogger("com.digitalpebble.stormcrawler.urlfrontier").setLevel(Level.TRACE);

looks good, I'll try it when I get home!

jnioche · 2022-07-06T10:22:27Z

Do you have an idea how we could test this properly?

I can reproduce the issue. Not sure why it is happening but it doesn't matter too much. If it can be solved later on, then it would be good but we can go ahead with this PR nonetheless

FelixEngl · 2022-07-06T11:46:15Z

Do you have an idea how we could test this properly?

I can reproduce the issue. Not sure why it is happening but it doesn't matter too much. If it can be solved later on, then it would be good but we can go ahead with this PR nonetheless

It is fixed with the other related PR. The fix already running in production (inhouse). But as you said, we can talk about it if we reach that point.

jnioche · 2022-07-07T10:50:47Z

@FelixEngl have merged it and will fix the couple of comments shortly. Thanks a lot for your work on this

Add unit test for ACKing.

04dc040

Signed-off-by: Felix Engl <felix.engl@uni-bamberg.de>

FelixEngl mentioned this pull request Jul 5, 2022

Overhaul urlfrontier-bolts, add tests, find problem with URLFrontier #980

Closed

Update PR like discussed in apache#982.

f8b1a4d

Signed-off-by: Felix Engl <felix.engl@uni-bamberg.de>

jnioche requested changes Jul 6, 2022

View reviewed changes

...nal/urlfrontier/src/test/java/com/digitalpebble/stormcrawler/urlfrontier/StatusBoldTest.java Outdated Show resolved Hide resolved

Merge branch 'DigitalPebble:master' into URLFrontier_AddFirstTest

2cf9fc5

FelixEngl and others added 2 commits July 6, 2022 10:16

Rename StatusBoldTest to StatusUpdaterBoltTest

897bc14

Signed-off-by: Felix Engl <felix.engl@uni-bamberg.de>

Merge remote-tracking branch 'origin/URLFrontier_AddFirstTest' into U…

1e43bd8

…RLFrontier_AddFirstTest

jnioche reviewed Jul 6, 2022

View reviewed changes

...frontier/src/test/java/com/digitalpebble/stormcrawler/urlfrontier/StatusUpdaterBoltTest.java Show resolved Hide resolved

jnioche reviewed Jul 6, 2022

View reviewed changes

jnioche added enhancement urlfrontier labels Jul 6, 2022

jnioche added this to the 2.5 milestone Jul 6, 2022

jnioche merged commit 7a622e5 into apache:master Jul 7, 2022

FelixEngl deleted the URLFrontier_AddFirstTest branch July 8, 2022 06:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit test basics for URLFrontier. #984

Add unit test basics for URLFrontier. #984

FelixEngl commented Jul 5, 2022

jnioche left a comment

FelixEngl commented Jul 6, 2022

jnioche Jul 6, 2022

jnioche Jul 6, 2022

FelixEngl Jul 6, 2022

jnioche Jul 6, 2022

FelixEngl Jul 6, 2022

jnioche commented Jul 6, 2022

FelixEngl commented Jul 6, 2022 •

edited

jnioche commented Jul 7, 2022

Add unit test basics for URLFrontier. #984

Add unit test basics for URLFrontier. #984

Conversation

FelixEngl commented Jul 5, 2022

Developer Certificate of Origin

jnioche left a comment

Choose a reason for hiding this comment

FelixEngl commented Jul 6, 2022

jnioche Jul 6, 2022

Choose a reason for hiding this comment

jnioche Jul 6, 2022

Choose a reason for hiding this comment

FelixEngl Jul 6, 2022

Choose a reason for hiding this comment

jnioche Jul 6, 2022

Choose a reason for hiding this comment

FelixEngl Jul 6, 2022

Choose a reason for hiding this comment

jnioche commented Jul 6, 2022

FelixEngl commented Jul 6, 2022 • edited

jnioche commented Jul 7, 2022

FelixEngl commented Jul 6, 2022 •

edited