Fix race in execution of tests

[jtorin]

Summary

Make the test itinerary instance non-static as it's modified in some tests.
Specifically, an itinerary can be flagged for deletion in one test, which affects the assertions in the next test.

Without this, tests may fail if they are executed in a certain order. This may be because LC_COLLATE is not set in the environment.

Similar code

Searching for this idiom in the tree:

private static final Itinerary INSTANCE = newItinerary( ... );

...reveals other usages, although that presently they do not cause build problems. But in the worst case, it could be that a test passes because a previously executed test provides the wanted state.

Bonus observation

Itinerary.isFlaggedForDeletion() calls getSystemNotices() which in turn makes a copy of the systemNotices collection. This seems wasteful and just generates GC churn.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.90%. Comparing base (15ba54a) to head (48399cc).
Report is 53 commits behind head on dev-2.x.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6177      +/-   ##
=============================================
- Coverage      69.93%   69.90%   -0.03%     
+ Complexity     17727    17722       -5     
=============================================
  Files           1996     1998       +2     
  Lines          75402    75443      +41     
  Branches        7717     7718       +1     
=============================================
+ Hits           52730    52740      +10     
- Misses         19995    20024      +29     
- Partials        2677     2679       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jtorin]

Updated PR with variable names reflecting that they are no longer static.

[t2gran]

Itinerary.isFlaggedForDeletion() calls getSystemNotices() which in turn makes a copy of the systemNotices collection. This seems wasteful and just generates GC churn.

Would you mind changing the isFlaggedForDeletion() it can safely be done in this PR:

  public boolean isFlaggedForDeletion() {
    return !systemNotices.isEmpty();
  }

[t2gran]

This looks ok, I would appreciate if you fixed the Itinerary.isFlaggedForDeletion() in this PR or in a sep PR. But, it is not required to merge the PR .

[jtorin]

This looks ok, I would appreciate if you fixed the Itinerary.isFlaggedForDeletion() in this PR or in a sep PR. But, it is not required to merge the PR .

I've added a one-liner commit that simplifies the Itinerary.isFlaggedForDeletion() implementation.