"... about the additional change in [1] and unit test is a good candidate for this particular npe..."

Exactly my point. In general a lot of NPE's could have been discovered with proper unittesting.

thanks,
Robert

Op Mon, 03 Aug 2015 11:30:21 +0200 schreef Anton Tanasenko <atg.sleepl...@gmail.com>:

I think Robert was talking about the additional change in [1] and unit test
is a good candidate for this particular npe, since it's basically a
programming error. Better to catch it early on.
I've added a test to the PR for the overall LifecyclePhase class
functionality which would also prevent this npe from happening in the
future.

[1] https://github.com/apache/maven/pull/62/files?w=1


2015-08-03 11:38 GMT+03:00 <herve.bout...@free.fr>:

yes, currently we consider only core ITs as core ITs [1]: but plugins
trunks are good core ITs candidates too :)

Regards,

Hervé

[1] https://builds.apache.org/view/M-R/view/Maven%20Core%20ITs/

----- Mail original -----
De: "Jason van Zyl" <ja...@takari.io>
À: "Maven Developers List" <dev@maven.apache.org>
Envoyé: Lundi 3 Août 2015 04:09:53
Objet: Re: [VOTE] Maven 3.3.6

I’m not sure a unit test really matters as it was a conscious change in a public signature which in and of itself is fine. If the signature changed
then a unit test would have to change accordingly. But unless we have
something akin to Clirr or a semantic version check then we don’t know how
this will impact consumers of the public API which is the problem
Karl-Heinz noticed with the enforcer. We have tests and they should have
detected the issue. Our CI is setup is not helping us detect these issues. This is what I’m going to work on this week in a way I think is effective.
I don’t think unit tests are of any help in this particular case but do
anything you think will help.

I’ll cut the release next Monday.

> On Aug 2, 2015, at 5:46 PM, Anton Tanasenko <atg.sleepl...@gmail.com>
wrote:
>
> Sure thing, I'll update pull request.
> Thanks for reminding.
>
> 2015-08-02 21:00 GMT+03:00 Robert Scholte <rfscho...@apache.org>:
>
>> Even though the changes seem too simple, I'd like to see unittests to
>> prevent this from happening again.
>> I don't mind writing them.
>>
>> Robert
>>
>> Op Sat, 01 Aug 2015 17:05:37 +0200 schreef Anton Tanasenko <
>> atg.sleepl...@gmail.com>:
>>
>>
>> I've created yet another PR for MNG-5805:
>>> https://github.com/apache/maven/pull/62
>>> Sorry guys, I was certain I had run m-enforcer-p its as well with 3.3.5
>>> candidate.
>>>
>>> 2015-08-01 16:57 GMT+03:00 Anton Tanasenko <atg.sleepl...@gmail.com>:
>>>
>>> Oh not again,
>>>> The LifecyclePhase#toString() method was added by me along with
>>>> toLegacyMap.
>>>> Looks like I didn't take care of 'mojos' field possibly being null.
This
>>>> seems to be happening when an extension specifies a lifecycle phase
but
>>>> doesn't assign any mojo executions to it.
>>>>
>>>>
>>>> 2015-08-01 16:08 GMT+03:00 Karl Heinz Marbaise <khmarba...@gmx.de>:
>>>>
>>>> Hi,
>>>>>
>>>>> +0 from me...
>>>>>
>>>>> Reference Documentation updated (
http://maven.apache.org/ref/3-LATEST/)
>>>>> ..
>>>>>
>>>>> checked with maven-invoker-plugin, maven-install-plugin, with Maven
>>>>> itself ;-)...
>>>>> and with some of my own projects without any issue...
>>>>>
>>>>>
>>>>> Unfortunately i have found an issue with maven-enforcer-plugin
>>>>> (trunk:r1693704) where two integrations test are failing with Maven
>>>>> 3.3.6
>>>>> which do not fail with Maven 3.3.3...
>>>>>
>>>>> I have attached both log files...
>>>>>
>>>>> Maven Enforcer accesses methods in maven-core which are marked as
>>>>> deprecated...(based on indirect changes):
>>>>>
>>>>> This has been added with 3.3.6...
>>>>>
>>>>>
>>>>>
./maven-core/src/main/java/org/apache/maven/lifecycle/mapping/LifecyclePhase.java
>>>>>
>>>>>
>>>>> The following is called indirectly...
>>>>>
>>>>>    @Deprecated
>>>>>    public static Map<String, String> toLegacyMap( Map<String,
>>>>> LifecyclePhase> lifecyclePhases )
>>>>>    {
>>>>>        if ( lifecyclePhases == null )
>>>>>        {
>>>>>            return null;
>>>>>        }
>>>>>
>>>>>        if ( lifecyclePhases.isEmpty() )
>>>>>        {
>>>>>            return Collections.emptyMap();
>>>>>        }
>>>>>
>>>>>        Map<String, String> phases = new LinkedHashMap<>();
>>>>>        for ( Map.Entry<String, LifecyclePhase> e:
>>>>> lifecyclePhases.entrySet() )
>>>>>        {
>>>>>            phases.put( e.getKey(), e.getValue().toString() );
>>>>>        }
>>>>>        return phases;
>>>>>    }
>>>>>
>>>>>
>>>>> So the culprit seemed to be this line:
>>>>>
>>>>>            phases.put( e.getKey(), e.getValue().toString() );
>>>>>
>>>>> where e.getValue() could be null...but they are not the problem.
>>>>>
>>>>> The problem is located in the toString() method of LifecyclePhase:
>>>>>
>>>>>   @Override
>>>>>    public String toString()
>>>>>    {
>>>>>        StringBuilder sb = new StringBuilder();
>>>>>        boolean first = true;
>>>>>        for ( LifecycleMojo mojo: getMojos() )
>>>>>        {
>>>>>            if ( first )
>>>>>            {
>>>>>                first = false;
>>>>>            }
>>>>>            else
>>>>>            {
>>>>>                sb.append( "," );
>>>>>            }
>>>>>            sb.append( mojo.getGoal() );
>>>>>        }
>>>>>        return sb.toString();
>>>>>    }
>>>>>
>>>>> The call to getMojos() is the real problem cause the call to
>>>>> "toLegacyMap()" is a static method without relationship to the object
>>>>> instance which could explain the non existings content of
getMojos()...
>>>>>
>>>>> This looks to me that maven-enforcer needed to be fixed...But WDYT ?
>>>>>
>>>>> Kind regards
>>>>> Karl Heinz Marbaise
>>>>>
>>>>>
>>>>> On 7/31/15 5:41 AM, Jason van Zyl wrote:
>>>>>
>>>>> Hi,
>>>>>>
>>>>>> Time to release Maven 3.3.6!
>>>>>>
>>>>>> Here is a link to the issues resolved:
>>>>>>
>>>>>>
>>>>>>
https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12316922&version=12333058
>>>>>>
>>>>>> Staging repo:
>>>>>> https://repository.apache.org/content/repositories/maven-1201/
>>>>>>
>>>>>> The distributable binaries and sources for testing can be found
here:
>>>>>>
>>>>>>
>>>>>>
https://repository.apache.org/content/repositories/maven-1201/org/apache/maven/apache-maven/3.3.6/
>>>>>>
>>>>>> Specifically the zip, tarball, and source archives can be found
here:
>>>>>>
>>>>>>
>>>>>>
https://repository.apache.org/content/repositories/maven-1201/org/apache/maven/apache-maven/3.3.6/apache-maven-3.3.6-bin.zip
>>>>>>
>>>>>>
>>>>>>
https://repository.apache.org/content/repositories/maven-1201/org/apache/maven/apache-maven/3.3.6/apache-maven-3.3.6-bin.tar.gz
>>>>>>
>>>>>>
>>>>>>
https://repository.apache.org/content/repositories/maven-1201/org/apache/maven/apache-maven/3.3.6/apache-maven-3.3.6-src.zip
>>>>>>
>>>>>>
>>>>>>
https://repository.apache.org/content/repositories/maven-1201/org/apache/maven/apache-maven/3.3.6/apache-maven-3.3.6-src.tar.gz
>>>>>>
>>>>>> Source release checksum(s):
>>>>>> apache-maven-3.3.6-src.zip sha1:
>>>>>> ae409472561584c50691e672539b3eb0f11e806a
>>>>>>
>>>>>> Staging site:
>>>>>> http://people.apache.org/~jvanzyl/maven-3.3.6/
>>>>>>
>>>>>> Vote open for 72 hours.
>>>>>>
>>>>>> [ ] +1
>>>>>> [ ] +0
>>>>>> [ ] -1
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> The Maven Team
>>>>>>
>>>>>>
>>>>>
>>>>> Kind regards
>>>>> Karl Heinz Marbaise
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
>>>>> For additional commands, e-mail: dev-h...@maven.apache.org
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Anton.
>>>>
>>>>
>>>
>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
>> For additional commands, e-mail: dev-h...@maven.apache.org
>>
>>
>
>
> --
> Regards,
> Anton.

Thanks,

Jason

----------------------------------------------------------
Jason van Zyl
Founder, Takari and Apache Maven
http://twitter.com/jvanzyl
http://twitter.com/takari_io
---------------------------------------------------------

Lastly, "Impossible." The lamest of the lame excuses! Difficult maybe, or
impractical, or too expensive, but rarely is anything impossible.

  -- Yvon Chouinard, Let my People Go Surfing













---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

Reply via email to