JCgH4164838Gh792C124B5 commented on pull request #458:
URL: https://github.com/apache/struts/pull/458#issuecomment-748524387


   Hi @Yilinwang , @yasserzamani and @lukaszlenart .
   
   Interesting topic.  😃 
   
   It looks like this PR and PR #457 have a commonality in that they both 
involve instrumentation of tests using the NonDex tool.  The tests pass 
successfully during a normal run, but some intermittent failures can appear if 
using NonDex (which instruments the code to change ordering behaviour, as I 
understand it).
   
   Some of the existing unit tests may function under the assumption of an 
implicit ordering of annotations.  It seems that such ordering may be stable, 
but not explicitly guaranteed by anything specification-wise.  Others may have 
better information or understanding on that topic.  😅 
   
   The "implicit" order of annotation processing seems to have been in place 
for some time and worked "OK so far", so applying an alphabetical sort (such as 
in this PR) to `AnnotationValidationConfigurationBuilder` may need some careful 
consideration.
   
   Applying an explicit order (alphabetical or otherwise) might provide some 
consistency, but could have unintended consequences.  Updating tests that 
assume a specific order, to handle different possible ordering, could be 
another approach, too. 
   
   Either way it might be a good idea to clarify the expectations in the 
documentation (such as "the order of the annotation processing is not 
guaranteed", or if it is changed, "the annotations will be processed in sort 
order X").


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to