Do not permit multiple settings files

[jasontedor]

This commit enforces that at most a single settings file is found. If
multiple settings files are found, a SettingsException will be thrown

Closes #13042

[rmuir]

It is a matter of taste i suppose, but i still hate all junit support for exceptions like this.

The rule sucks because it has side effects, if we write another test that uses it, it can easily have some bogus leftover state from a previous test method?

Personally i still do it tests like this:

try {
  something();
  fail("should have hit expected exception");
} catch (SomeException expected) {
  assertTrue(expected.getMessage().contains("expected text"));
}

This sucks too, in that its easy to forget the fail() part and have the whole test do nothing.

[jasontedor]

The rule sucks because it has side effects, if we write another test that uses it, it can easily have some bogus leftover state from a previous test method?

I don't think so. I think that @rule is just like @before in that it's processed once before each test method thus setting the state to empty before each test method runs. I'll research.

[rmuir]

Here would be something of an alternative maybe for the future, when java 8 is minimal (it would be less annoying due to effectively final and lambda):

public void test() {
    assertError(IndexOutOfBoundsException.class, () -> {
        int foo[] = new int[5];
        System.out.println(foo[6]);
    });
}

// test helper for expected exception
// TODO: can we replace Runnable with TestMethod or similar interface that throws Throwable
// so checked exceptions arent annoying?
static void assertError(Class<? extends Throwable> expectedClazz, Runnable foo) {
    assertError(expectedClazz, null, foo);
}

// test helper for expected exception, with expected message
static void assertError(Class<? extends Throwable> expectedClazz, String expectedMessage, Runnable foo) {
    try {
        foo.run();
        fail("didnt hit expected exception, expected: " + expectedClazz.getSimpleName());
    } catch (Throwable t) {
        if (!expectedClazz.isAssignableFrom(t.getClass())) {
            throw new IllegalStateException("got the wrong exception, expected: " + expectedClazz.getSimpleName() + ", got: " + t, t);
        }
        if (t.getMessage() == null && expectedMessage != null) {
            throw new IllegalStateException("exception had null message, but expected: " + expectedMessage, t);
        }
        if (expectedMessage != null && !t.getMessage().contains(expectedMessage)) {
            throw new IllegalStateException("expection message did not contain expected text: " + expectedMessage, t);
        }
    }
}

[jasontedor]

The execptedException field will be initialized before each test method.

[rmuir]

ok, then i am fine with it! Maybe we should followup and consider moving it to a base class and we do this stuff in a consistent way everywhere. I bet we will find test bugs.

[rmuir]

And one downside even with being initialized before each test method, is some tests do multiple asserts like this inside a single method. So we'd have to clean up those tests to be separate test methods, but thats still fine.

[jpountz]

Similar concerns as Robert for tests, but otherwise LGTM