Spark3.5: Standardizing Error Handling in Iceberg Spark Module - TestViews

[huaxingao]

Changing error validation from checking error message to validating error classes in Spark module, starting from TestViews. Here is the original discussion.

[nastra]

I would prefer if we could keep the error msg check. I think it's perfectly fine to keep this and then just add the .satisfies check below

[nastra]

If we really only care about the error class, then an alternative check could be the one below:

assertThatThrownBy(() -> sql("SELECT * FROM %s", viewName))
        .isInstanceOf(AnalysisException.class)
        .hasMessageContaining(
            String.format(
                "The table or view `%s`.`%s`.`non_existing` cannot be found",
                catalogName, NAMESPACE))
        .asInstanceOf(InstanceOfAssertFactories.type(AnalysisException.class))
        .extracting(AnalysisException::getErrorClass)
        .isEqualTo("INVALID_VIEW_TEXT");

[huaxingao]

Thanks @nastra for your comment! I'm concerned that checking for specific error messages might make the tests a bit fragile. While this particular error message seems stable, others might change between versions. What do you think?

[nastra]

I'd say let's keep the error messages for now until we actually see that those become more difficult to maintain because they change more frequently (which I actually don't expect to happen). The reason I'd like to keep the error msg is to make sure that we properly ensure and see what a typical end user would see when a certain error condition happens