[resteasy-dev] RESTEasy and possible "community" contribution

Wed May 4 17:25:30 EDT 2016

Hi Rostislav,
thanks for having started the discussion; below some replies...

Il 04/05/2016 18:11, Rostislav Svoboda ha scritto:
>>> Moving the discussion to the mailing list ++ adding few notes about the
>>> project structure and TS.
>>>
>>> Structure:
>>> - why is there one module jaxrs in the root of repo -
>>> https://github.com/resteasy/Resteasy ?
>> It's added by Bill but I forget the reason. I don't like it either :-) We can
>> remove it now as JAX-RS is moved outside RESTEasy repo.
+1
>>
>>> - there are too many modules (in root jaxrs module) from my perspective
>>>    -- multi module project structure can affect build time
>>>    -- find . | grep pom.xml | wc -l reports 226 pom.xml files
>> The 'example' module contains all the Bill's book examples. We need to ask
>> him for permission to move these away.
> I think it can't cause any harm if it's just moved to separate repo under https://github.com/resteasy
I don't think we actually need an additional repo for the examples and 
books stuff, what we really need (as you actually already pointed out) 
is having them clearly isolated, which means:
* everything under 'examples' dir
* clear maven group and artifact naming (let's say, anything in there 
should have a "resteasy-examples-" prefix or something like that)
That way we can keep on including the examples etc in the distro while 
reducing clutter in the IDE (you simply avoid including the examples 
stuff when importing the project and in any case the examples maven 
modules are all close together and in order).

>
> Commits:
> - commit messages do not follow same/similar format - see
> https://github.com/resteasy/Resteasy/commits/master
>    -- I would expect jira id at the beginning of the commit message, it's
>    there sometime but in different format - e.g. RESTEASY-1328 vs.
>    [RESTEASY-1331]
> - massive commit message like
> https://github.com/resteasy/Resteasy/commit/fdd1f9f31edb894fa6f8684f2608224c39519e6c
> - commit related to RESTEASY-1323 are really "fun"
>    -- https://github.com/resteasy/Resteasy/pull/756/files
>    -- one fix in code (4 lines removed) + two tests done in 7 commits :(
>    -- these commits should have been squashed
Yeah, it should be easy to be a little bit more accurate with the commit 
messages. As for the jira reference, it should always be like 
"[RESTEASY-XYZ]" and there's no actual need to include the jira URL as 
jira can be configured to automatically parse references in commit 
messages to link commits to related issues.

>>> Versions:
>>> - will leave this to Tomaz :)
>>>
>>> TS:
>>> - unit tests are mixed with integration tests
>>>    -- integration tests should be in separate module
>>>    -- tests should be running in different maven phases
>>> - there are only few tests against WF
>>> - such tests are not executed against latest WF, but mainly against WF 8
>>> - I would like to see ARQ + WF10+ and get rid of tjws
>> Get ridding of TJWS is not practical because it will make the test time
>> unacceptable. But replacing them with undertow container would be great :-)
> Do you have estimation how much time tjws saves ? And PRs are processed on external system (Travis atm) so I wouldn't be bothered if the TS runs for few minutes longer.
>
> As proven by EAP QE internal resteasy testsuite, it's not revealing issues which could be eliminated before RESTEasy goes into the product.
>
> For me Undertow container is not sufficient from product perspective, WF is needed.
I'm also for using WildFly as the default and preferred testing 
container, unless an issue related to a different container integration 
is being worked/tested. I would even propose to do something like what 
we do in JBossWS, that is test against the latest 3 final releases of 
WildFly + current master (to anticipate integration issues).
The project build should be able to build the required artifacts, 
install them on the specified wildfly target instance, start it and run 
the testsuite against it.
While starting a container instance for each (integration)test and 
stopping it as soon as the test is over is nice, if you think about that 
it's not really required. We should always ensure tests are idempotent, 
that is they leave the target container unchanged after their execution; 
once that's granted, we can run multiple tests against the same server, 
no need to keep on starting and stopping it, which is what really takes 
time. With a proper deployment name isolation, it should also be 
possible to run multiple tests at the same time against the same target 
server.
Ideally (I understand that might not be doable for any type of test), it 
would really be cool to able to select the target container among wfly 
and other available integrations (jetty, tjws, etc), so that we only 
maintain a set of tests and it's then up to the CI system to verify they 
pass against all supported targets (a run for each target, perhaps 
specifying it with a different profile)

>
>>>
>>> EAP 7 situation for TS:
>>> - we migrated upstream ts to use ARQ and run against EAP 7 and not tjws
>>> - we discovered half of reported issues thanks to this migrated TS, these
>>> issues wouldn't be noticed bu community TS executed against tjws
>> Totally removing TJWS is also not possible, because many users are using this
>> 'internal' feature in their production env. We can't estimate how the impact
>> will be.
Sure, but if we feel that's the right thing to do, we can deprecate the 
tjws integration in a 3.0.x version and remove it in next major.

>>
>> We can try to remove TJWS for testing purposes in RESTEasy project itself.
> +100

Yes, but see above
>
>> But the RESTEasy TJWS module should be maintained as a feature.
> I think it should be moved into separate repository with own release cycle. Not even sure if it should be under https://github.com/resteasy, but for now it would be sufficient ... I guess
We *could* have the integrations live in different components/repos, 
similarly to what we have in jbossws, but that comes with release and 
maintenance costs, so I won't do that unless there's real benefit in the 
move.

>>>    -- 19 from 38 jiras -
>>>    https://issues.jboss.org/issues/?jql=project%20%3D%20JBEAP%20AND%20issuetype%20%3D%20Bug%20AND%20resolution%20in%20%28Unresolved%2C%20Done%2C%20%22Out%20of%20Date%22%29%20AND%20component%20%3D%20REST
>>> - we will keep this TS for 7.0.z
>>> - we would like to consume & prod-patch upstream (ARQ + WF based) TS for
>>> the future releases
>>>
>>> Upstream and QE specific TS should somehow converge into a single one.
>> +1
> This can only happen if the TS runs against WF as EAP is based on WF.
> ARQ here for some time and it is really preferred from our side.
I agree on both the WF target reasoning and on using Arquillian.

Finally, not sure where to comment on this, anyway I won't isolate the 
TS too much from the rest of the code. I agree on moving the integration 
tests to dedicated project module(s), but those should live in the same 
repo and by triggered by the same build of the resteasy core.

Cheers
Alessio

-- 
Alessio Soldano
Web Service Lead, JBoss