brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #174: pr173/rename confMapThing.obj
Date Thu, 02 Jun 2016 10:49:54 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/174#discussion_r65519069
  
    --- Diff: core/src/test/java/org/apache/brooklyn/core/entity/EntityConfigTest.java ---
    @@ -208,8 +210,18 @@ public void testGetConfigMapWithExplicitMap() throws Exception {
             assertEquals(entity.config().getNonBlocking(TestEntity.CONF_MAP_THING.subKey("mysub")).get(),
"myval");
         }
         
    -    @Test
    -    public void testGetConfigNonBlocking() throws Exception {
    +    // TODO This now fails because the task has been cancelled, in entity.config().get().
    +    // But it used to pass (e.g. with commit 56fcc1632ea4f5ac7f4136a7e04fabf501337540).
    +    // It failed after the rename of CONF_MAP_THING_OBJ to CONF_MAP_OBJ_THING, which

    +    // suggests there was an underlying problem that was masked by the unfortunate naming
    +    // of the previous "test.confMapThing.obj".
    +    //
    +    // Presumably an earlier call to task.get() timed out, causing it to cancel the task?
    +    // I (Aled) question whether we want to support passing a task (rather than a 
    +    // DeferredSupplier or TaskFactory, for example). Our EntitySpec.configure is overloaded
    +    // to take a Task, but that feels wrong!?
    --- End diff --
    
    @neykov you're right - we support passing any of Task, Future and DeferredSupplier. I
agree we should continue to support those, and should fix whatever is cancelling this task.
I think it's probably to do with the implementation of `MapConfigKey`, given the test only
broke after the rename (which meant that "obj" couldn't be mistaken for a sub-key of the other
MapConfigKey called "test.confMapThing".
    
    I also see that `MapConfigKey.applyEntryValueToMap` does an instanceof check for `Supplier`,
but that `ValueResolver` does not - that seems wrong.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message