Add new turtle test node based directly on NodeBase #353

gavin-norman-sociomantic · 2018-08-02T14:56:43Z

(Currently based on #350. Only the last commit is relevant.)

gavin-norman-sociomantic · 2018-08-02T15:05:16Z

I believe the separation of turtle node from the actual node implementation is a historical artefact from the days when the fake nodes lived partially in turtle. It seems to be unnecessary now.

gavin-norman-sociomantic · 2018-08-02T15:07:30Z

sociomantic-tsunami/dlsproto#101 gives a rough example of how this change simplifies the code for a fake node.

mihails-strasuns · 2018-08-03T10:03:51Z

Hm, this doesn't look like any code I have written and turtle definitely doesn't care so I'd say "whatever fits you best" :)

gavin-norman-sociomantic · 2018-08-03T10:04:33Z

It was derived from code in turtle that you wrote originally.

mihails-strasuns · 2018-08-03T10:05:33Z

I pretty sure that it was you who wrote that code in turtle originally :) I copied it to swarm with almost no changes when splitting things.

mihails-strasuns · 2018-08-03T10:08:30Z

src/turtle/env/model/TestNode.d

+        Notes:
+            1. Restarting the node *does not* clear any data in its storage
+               engine. To do that, call clear().
+            2. You must call stop() first, before calling restart().


This is extremely weird tbh. If you need to call stop first, it is just start, not restart.

Agreed. I have no memory of why we did it like this.

src/turtle/env/model/TestNode.d

gavin-norman-sociomantic · 2018-08-03T10:12:48Z

I pretty sure that it was you who wrote that code in turtle originally :) I copied it to swarm with almost no changes when splitting things.

Could be :) Mainly I'm interested in your opinion on the concept, and whether I'm missing some subtlety of turtle.

mihails-strasuns · 2018-08-09T11:53:50Z

AFAIR turtle only cares that nodes stop printing messages once unregister is called and everything else is completely up to you.

gavin-norman-sociomantic · 2018-08-09T15:28:39Z

Adapted with working restart. (Still super rough.)

gavin-norman-sociomantic · 2018-08-14T09:34:21Z

Tidied up.

gavin-norman-sociomantic · 2018-08-14T09:40:26Z

D2 fixes.

gavin-norman-sociomantic · 2018-08-14T09:41:24Z

Still TODO:

Release notes (new feature / deprecation).
Decide whether the changes to NodeBase are breaking, and whether there's a non-breaking way of doing them.

src/swarm/node/model/NeoNode.d

src/turtle/env/model/Node.d

src/swarm/node/model/NeoNode.d

nemanja-boric-sociomantic · 2018-08-22T14:21:17Z

src/turtle/env/model/TestNode.d

+
+    static this ( )
+    {
+        log = Log.lookup("testnode");


Hmmm. Is there a way for the fake node implementation to override the testnode log name? It's useful when testing with multiple types of test nodes.

Yeah, I'll change it to one logger per instance.

nemanja-boric-sociomantic · 2018-08-22T14:21:52Z

src/turtle/env/model/TestNode.d

+        ConnectionSetupParams conn_setup_params, Options options, int backlog )
+    {
+        verify(!already_created, "Can only have one " ~ idup(this.id) ~
+            " per turtle test app");


This is a case for assert?

nemanja-boric-sociomantic · 2018-08-22T14:24:32Z

src/turtle/env/model/TestNode.d

+
+        this.register(theScheduler.epoll);
+        if ( Task.getThis() )
+            theScheduler.processEvents();


Is there a case where this is not running in task? With the turtle, I don't think so, but I might be mistaken.

(note that you require that theScheduler.epoll is something - perhaps that's worth verifying as well?)

nemanja-boric-sociomantic · 2018-08-22T14:26:09Z

src/turtle/env/model/TestNode.d

+
+    ***************************************************************************/
+
+    abstract public void clear ( );


Hm, what's the reason for having this abstract method?

True, it's not required at this level.

nemanja-boric-sociomantic · 2018-08-22T14:35:21Z

integrationtest/turtleenv/main.d

+            ( Client.Neo.Get.Notification, Const!(Client.Neo.Get.Args) ) { });
+        if ( !ok )
+            return false;
+


if (value[] != "hello") return false;

nemanja-boric-sociomantic · 2018-08-22T14:54:49Z

Needs rebase now.

gavin-norman-sociomantic · 2018-08-23T10:50:25Z

Rebased.

nemanja-boric-sociomantic · 2018-08-24T12:37:51Z

Hmmmm

./src/swarm/node/model/Node.d(702): Error: class swarm.node.model.Node.TestNode interface function INode.restartListeners isn't implemented
./src/swarm/node/model/Node.d(702): Error: class swarm.node.model.Node.TestNode interface function INode.restartListeners isn't implemented
./src/swarm/node/model/Node.d(702): Error: class swarm.node.model.Node.TestNode interface function INode.restartListeners isn't implemented```

nemanja-boric-sociomantic · 2018-08-24T12:38:35Z

I also left a review on this PR :-)

gavin-norman-sociomantic · 2018-08-24T12:52:19Z

Ohh shit. I changed INodeBase, but only adapted the neo node classes that implement that interface :/

References to the listeners are required by INodeBase, but these were previously passed into the ctor from outside, rather than being owned and controlled by this class. This, combined with the lack of support in ocean for reopening a shutdown listener, made it impossible to implement a method to restart the node's listeners. (The ideal solution, of course, would be to add the missing restart support to the select listener in ocean, but this will require a major swarm release.)

(As a counterpart to the existing stopListener.)

There's more than one listener now.

This is intended as a replacement of the existing turtle.env.model.Node. The reason for the new class is that the turtle/swarm node framework is currently fairly complicated, with several different classes required to implement a fake node. The new TestNode derives directly from NodeBase (the standard base class for swarm nodes), whereas the older Node does not. This means that to implement a fake node using TestNode, one does not have to separately write a class deriving from NodeBase.

gavin-norman-sociomantic · 2018-08-24T14:34:56Z

Updated.

mihails-strasuns · 2018-12-03T11:04:53Z

@gavin-norman-sociomantic was there anything left for this PR from your side? It looks fine to me but there is a plenty of neo stuff I have no idea about :)

gavin-norman-sociomantic · 2018-12-06T14:52:09Z

The PR looks sensible to me, in isolation. I think it was waiting on trying to adapt one of the proto fake nodes, to confirm that it works (and is helpful) in practice.

gavin-norman-sociomantic · 2018-12-13T13:16:18Z

Ah, this appears to be a test of adapting a *proto to use this: sociomantic-tsunami/dlsproto#101

gavin-norman-sociomantic added type-refactoring part-node labels Aug 2, 2018

gavin-norman-sociomantic added this to the v5.3.0 milestone Aug 2, 2018

gavin-norman-sociomantic requested review from nemanja-boric-sociomantic and mihails-strasuns August 2, 2018 15:03

gavin-norman-sociomantic mentioned this pull request Aug 2, 2018

[WIP] Factor out fakedls.DlsNode sociomantic-tsunami/dlsproto#101

Open

gavin-norman-sociomantic force-pushed the nodeenv branch from 96304fc to 4869547 Compare August 2, 2018 15:15

mihails-strasuns reviewed Aug 3, 2018

View reviewed changes

src/turtle/env/model/TestNode.d Show resolved Hide resolved

gavin-norman-sociomantic force-pushed the nodeenv branch from 4869547 to 806a972 Compare August 8, 2018 09:42

gavin-norman-sociomantic force-pushed the nodeenv branch from 806a972 to 91403ab Compare August 9, 2018 15:27

gavin-norman-sociomantic force-pushed the nodeenv branch from 91403ab to c159506 Compare August 14, 2018 09:34

gavin-norman-sociomantic force-pushed the nodeenv branch from c159506 to 9e3915a Compare August 14, 2018 09:40

gavin-norman-sociomantic force-pushed the nodeenv branch from 9e3915a to e1d4868 Compare August 14, 2018 10:23

gavin-norman-sociomantic mentioned this pull request Aug 22, 2018

Replace internal usage of NodeItem with AddrPort #348

Closed

gavin-norman-sociomantic changed the title ~~[WIP] Add new turtle test node based directly on NodeBase~~ Add new turtle test node based directly on NodeBase Aug 22, 2018

nemanja-boric-sociomantic reviewed Aug 22, 2018

View reviewed changes

gavin-norman-sociomantic force-pushed the nodeenv branch from e1d4868 to 2563071 Compare August 23, 2018 10:49

Remove unused import

08afa14

gavin-norman-sociomantic force-pushed the nodeenv branch from 2563071 to 49137d7 Compare August 23, 2018 10:52

gavin-norman-sociomantic force-pushed the nodeenv branch from 49137d7 to b3c7934 Compare August 24, 2018 13:51

Gavin Norman added 5 commits August 24, 2018 16:34

Add NodeBase.restartListeners

003e1d3

(As a counterpart to the existing stopListener.)

Rename NodeBase.stopListener -> stopListeners

22e2b20

There's more than one listener now.

Adapt the turtle env node test to use the new TestNode

62c2478

gavin-norman-sociomantic force-pushed the nodeenv branch from b3c7934 to 62c2478 Compare August 24, 2018 14:34

gavin-norman-sociomantic modified the milestones: v5.3.0, v5.4.0 Nov 20, 2018

gavin-norman-sociomantic modified the milestones: v5.4.0, v5.5.0 Apr 11, 2019

Geod24 changed the base branch from v5.x.x to v6.x.x August 13, 2020 10:24

Geod24 removed this from the v5.5.0 milestone Aug 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new turtle test node based directly on NodeBase #353

Add new turtle test node based directly on NodeBase #353

gavin-norman-sociomantic commented Aug 2, 2018

gavin-norman-sociomantic commented Aug 2, 2018 •

edited

Loading

gavin-norman-sociomantic commented Aug 2, 2018

mihails-strasuns commented Aug 3, 2018

gavin-norman-sociomantic commented Aug 3, 2018

mihails-strasuns commented Aug 3, 2018

mihails-strasuns Aug 3, 2018

gavin-norman-sociomantic Aug 3, 2018

gavin-norman-sociomantic commented Aug 3, 2018

mihails-strasuns commented Aug 9, 2018

gavin-norman-sociomantic commented Aug 9, 2018

gavin-norman-sociomantic commented Aug 14, 2018

gavin-norman-sociomantic commented Aug 14, 2018

gavin-norman-sociomantic commented Aug 14, 2018

nemanja-boric-sociomantic Aug 22, 2018

gavin-norman-sociomantic Aug 24, 2018

nemanja-boric-sociomantic Aug 22, 2018

gavin-norman-sociomantic Aug 24, 2018

nemanja-boric-sociomantic Aug 22, 2018

nemanja-boric-sociomantic Aug 22, 2018

nemanja-boric-sociomantic Aug 22, 2018

gavin-norman-sociomantic Aug 24, 2018

nemanja-boric-sociomantic Aug 22, 2018

nemanja-boric-sociomantic commented Aug 22, 2018

gavin-norman-sociomantic commented Aug 23, 2018

nemanja-boric-sociomantic commented Aug 24, 2018

nemanja-boric-sociomantic commented Aug 24, 2018

gavin-norman-sociomantic commented Aug 24, 2018

gavin-norman-sociomantic commented Aug 24, 2018

mihails-strasuns commented Dec 3, 2018

gavin-norman-sociomantic commented Dec 6, 2018 •

edited

Loading

gavin-norman-sociomantic commented Dec 13, 2018


		***************************************************************************/

		abstract public void clear ( );

Add new turtle test node based directly on NodeBase #353

Are you sure you want to change the base?

Add new turtle test node based directly on NodeBase #353

Conversation

gavin-norman-sociomantic commented Aug 2, 2018

gavin-norman-sociomantic commented Aug 2, 2018 • edited Loading

gavin-norman-sociomantic commented Aug 2, 2018

mihails-strasuns commented Aug 3, 2018

gavin-norman-sociomantic commented Aug 3, 2018

mihails-strasuns commented Aug 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavin-norman-sociomantic commented Aug 3, 2018

mihails-strasuns commented Aug 9, 2018

gavin-norman-sociomantic commented Aug 9, 2018

gavin-norman-sociomantic commented Aug 14, 2018

gavin-norman-sociomantic commented Aug 14, 2018

gavin-norman-sociomantic commented Aug 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nemanja-boric-sociomantic commented Aug 22, 2018

gavin-norman-sociomantic commented Aug 23, 2018

nemanja-boric-sociomantic commented Aug 24, 2018

nemanja-boric-sociomantic commented Aug 24, 2018

gavin-norman-sociomantic commented Aug 24, 2018

gavin-norman-sociomantic commented Aug 24, 2018

mihails-strasuns commented Dec 3, 2018

gavin-norman-sociomantic commented Dec 6, 2018 • edited Loading

gavin-norman-sociomantic commented Dec 13, 2018

gavin-norman-sociomantic commented Aug 2, 2018 •

edited

Loading

gavin-norman-sociomantic commented Dec 6, 2018 •

edited

Loading