yes - the values aren't portable. however, the issue here is that the >behavior< isn't portable as well. e.g.: if you have owb specific properties (which need to be in openwebbeans.properties) and you try to pass them in via PropertiesProvider/TestProperties, you don't get a container which is aware of those properties.
the only way i would be ok with such a suggestion is that we log a warning if there are arguments passed to OpenWebBeansContainerControl#boot(Map) and WeldContainerControl#boot(Map) and we add a config (e.g. TestControlConfig#validatePortability). -> the startup would fail, if it is true (= default) and you pass properties to a container-adapter which can't support it. regards, gerhard 2014-09-20 19:40 GMT+02:00 Karl Kildén <[email protected]>: > Also, sorry that I started a new thread not mentioning Deltaspike-557[1] my > suggestion there is worse but the discussion was interesting. I tried some > of the proposed workaround and they were not good for me. i forgot I opened > that jira for some reason... I ended up working around it by not testing a > certain thing but now I must have it. > > In there I noticed that Romain agrees at least: "well it is portable but > values are not." > > Anyways I probably should have left the subject already as I said I would > and I will do so now. I already replaced Test-Control and it didn't even > take long since the api is so nice and concise. Anyway thanks I had a good > run with it ;) > > cheers > > [1] https://issues.apache.org/jira/browse/DELTASPIKE-577 > > > > On 20 September 2014 19:26, Karl Kildén <[email protected]> wrote: > > > If it was TomEEConfig.java rather than Properties I would understand your > > objection. However I think boot(Properties) is nice design regardless of > 0 > > or 5 end usages. Deltaspike is built with modules and CdiContainer is > just > > supposed to be an interface for someone who wan'ts to build an > > implementation. > > > > To offer boot(Properties) is hardly outrageous. Any Interface that is > > supposed to be a generic boot interface should offer some way to send in > > args. I can't really think of a better way than boot(Properties). > > > > I think it's fundamentally wrong to think only in means of what the > > majority of the implementations of today use. Reasonable api design in > > deltaspike could drive the development in cdi containers forward. When I > > googled for other embedded EJBContainers I found evidence supporting > > properties there too. > > > > Are you sure someone adding a module for glassfish / wildfly users would > > not like that boot(properties)? This was in my first search hit looking > at > > other containers: [1] > > > > / Create the container instance, passing it the properties map: > > EJBContainer container = > javax.ejb.embeddable.EJBContainer.createEJBContainer(*properties*); > > > > > > For Java EE users EJB and CDI containers blend together anyways. > > > > > > [1] https://netbeans.org/kb/docs/javaee/javaee-entapp-junit.html > > > > On 20 September 2014 19:00, Gerhard Petracek <[email protected] > > > > wrote: > > > >> yes and no - the initial situation for "main (args)" isn't the same. > >> once you create the same initial situation as a thought experiment, it > is > >> an example against your statement. > >> > >> the following is a >thought experiment< > >> to get a comparable case we would need to assume that all jvms > >> support proprietary config-files >instead< of parameters (for the > >> main-method). > >> furthermore, there is just one which supports both (proprietary > >> config-files as well as parameters). > >> -> you wouldn't use the method which is just provided by one of them, > >if< > >> you like to provide a >portable< app-starter. > >> > >> translate it to our discussion: > >> CdiContainer#boot(Map) does not work the same way for all containers, > >> because all containers (we support) except openejb ignore whatever you > >> pass > >> in. > >> -> CdiContainer#boot(Map) is controversial, but that doesn't mean that > >> other parts of deltaspike should start to ignore portability as well. > >> > >> we could move ContainerAwareTestContext to the spi which would allow to > >> customize different areas. > >> then you can decide on your own if you would like to use an > implementation > >> which isn't portable. > >> > >> regards, > >> gerhard > >> > >> > >> > >> 2014-09-20 16:47 GMT+02:00 Karl Kildén <[email protected]>: > >> > >> > The contract resides in Deltaspike: CdiContainer. Offering the > >> > implementations a way to receive properties is a general thing. It is > >> > countless times in countless apis - you know "main (args)" I > assume... > >> Any > >> > api that boots something should take args (imo). That some of the > >> > underlying apis does not want properties is not the same is "not > >> portable". > >> > Since that extra boot(Properties p) does not break those who do not > care > >> > for properties. > >> > > >> > Sounds almost like someone suggested removing boot() and only having > >> > boot(Properties p) > >> > > >> > Anyways I am done discussing this and I am fine with using my fork. > >> > > >> > I agree that not using test-control is a good idea and I will migrate > >> away > >> > from it. > >> > > >> > On 20 September 2014 16:30, Gerhard Petracek < > >> [email protected]> > >> > wrote: > >> > > >> > > it's just not portable since only tomee supports it. > >> > > tomee provides something very similar to our test-control, but > >> specific > >> > to > >> > > features provided by tomee. > >> > > imo it doesn't make sense to add something only for one container > (to > >> the > >> > > api) which is supported by the test-module of that container > already. > >> > > > >> > > regards, > >> > > gerhard > >> > > > >> > > > >> > > > >> > > 2014-09-20 16:24 GMT+02:00 Karl Kildén <[email protected]>: > >> > > > >> > > > FYI Gerhard said on the list that the boot(Properties p) in > >> > CdiContainer > >> > > > was a mistake and supporting it in test-control is thus wrong. > >> > > > > >> > > > I disagree and will branch test-control over n out > >> > > > > >> > > > On 20 September 2014 16:15, Karl Kildén <[email protected]> > >> wrote: > >> > > > > >> > > > > All those suggestions use properties so I am not sure what to > say > >> ;) > >> > > > > > >> > > > > On 20 September 2014 16:07, Gerhard Petracek < > >> > > [email protected] > >> > > > > > >> > > > > wrote: > >> > > > > > >> > > > >> hi karl, > >> > > > >> > >> > > > >> it sounds better than DELTASPIKE-577, however, please provide > the > >> > > > >> use-case/s which can't be done with [1]. > >> > > > >> (the other implementations we support right now don't support > >> such > >> > > > >> properties anyway). > >> > > > >> > >> > > > >> regards, > >> > > > >> gerhard > >> > > > >> > >> > > > >> [1] http://tomee.apache.org/alternate-descriptors.html > >> > > > >> > >> > > > >> > >> > > > >> > >> > > > >> 2014-09-20 15:28 GMT+02:00 Karl Kildén <[email protected] > >: > >> > > > >> > >> > > > >> > Hello, > >> > > > >> > > >> > > > >> > Test-Control will bootstrap the CdiContainer for me using the > >> > > #boot() > >> > > > >> > constructor. However I want it to use #boot(Properties p) > >> > > > >> > > >> > > > >> > This seems logical since CdiContainer contract has that boot > >> > method. > >> > > > My > >> > > > >> > suggestion is: > >> > > > >> > > >> > > > >> > public interface PropertiesProvider { > >> > > > >> > > >> > > > >> > Properties properties(); > >> > > > >> > } > >> > > > >> > > >> > > > >> > @TestControl(propertiesProvider=PropertiesProviderImpl.class) > >> > > > >> > > >> > > > >> > > >> > > > >> > Class<? extends PropertiesProvider> providerClazz = > >> > > > >> > this.testControl.propertiesProvider(); > >> > > > >> > if (providerClazz != null) { > >> > > > >> > Properties properties = > >> > providerClazz.newInstance().properties(); > >> > > > >> > } > >> > > > >> > > >> > > > >> > > >> > > > >> > All user have to do is implement that interface > >> PropertiesProvider > >> > > > and > >> > > > >> > assign it to the test. > >> > > > >> > > >> > > > >> > This would save me a lot of trouble... > >> > > > >> > > >> > > > >> > cheers > >> > > > >> > > >> > > > >> > >> > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > > > >
