Update test case ButterflyFacadeImplTest

[badalsarkar]

Hi Fabio, I noticed that the following test case is failing for assertion at line 104. This is because the order of the invalid keys are different. I ran it today and the actual result is coming as [, , 1a, a, b%, -a, #a]. https://github.com/paypal/butterfly/blob/df9e46367fa2cf1b6cc870a2eb411f900bbf22cd/butterfly-core/src/test/java/com/paypal/butterfly/core/ButterflyFacadeImplTest.java#L86-L105

We discussed about it here where the tests at my local machine and Travis CI passed with actual value as [1a, , a, -a, b%, #a, ]. Now the actual value is coming differently. I know this is because HashTable doesn't keep the order of insertion. But then why would it pass then and not now? This seems weird to me.

I want to update the test case as following. I am iterating over the properties with the same mechanism as in actual function call. Do you think it will mitigate the issue?

try {
            Properties properties = new Properties();
            properties.put("", "v1");
            properties.put(" ", "v2");
            properties.put("1a", "v3");
            properties.put("-a", "v4");
            properties.put("#a", "v5");
            properties.put("a", new Object());
            properties.put("b%", "v6");
            invalidProperties = properties.entrySet().stream()
                    .map(e -> (String) e.getKey())
                    .collect(Collectors.toList());

             butterflyFacadeImpl.newConfiguration(properties);
            fail("IllegalArgumentException was supposed to be thrown due to invalid properties");
        } catch (IllegalArgumentException e) {
            assertEquals(e.getMessage(), "The following properties are invalid: "+invalidProperties);
        }

[fabiocarvalho777]

Hello,

I think that still would not guarantee the order would always be the same. As an alternative solution, let's sort it here, before collection.

Usually I don't like changing code to fit tests, but in this case this is a very small change and with very small effect in runtime. With that we can reliably use the static String in the exception message, but making sure it matches the sorted order.

Does it make sense? Thanks.

[badalsarkar]

Yes. perfect. :smile:

Thank you.

[fabiocarvalho777]

@badalsarkar Thanks for fixing this issue!! :-)