search

trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
10.28k stars 2.96k forks source link

Replace usage of System.out/err from tests to reduce flakiness due to VM crashes #9107

Closed hashhar closed 1 month ago

hashhar commented 3 years ago

Surefire when running tests in forked VMs doesn't expect things to be written to System.out or System.err.

Surefire uses the sysout and syserr streams to "talk" to the forked VMs and doesn't expect other processes to write to those.

See https://stackoverflow.com/questions/55272870/surefire-maven-plugin-corrupted-stdout-by-directly-writing-to-native-stream-in for more details.

e.g. https://github.com/starburstdata/starburst-enterprise/blob/5c4bf97b0182e1b99fc5bb8[…]sto/plugin/hive/benchmark/BenchmarkStarburstHiveFileFormat.java is one of the most common reasons for why hive-hadoop2 tests are often failing with this error now.

We should investigate tests that fail with org.apache.maven.surefire.booter.SurefireBooterForkException: The forked VM terminated without properly saying goodbye. VM crash or System.exit called? for whether they are using System.out or System.err and fix them.

As pointed out by Piotr our TestNG listener also does this - see https://github.com/trinodb/trino/blob/master/testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java

findepi commented 3 years ago

As pointed out by Piotr our TestNG listener also does this - see https://github.com/trinodb/trino/blob/master/testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java

We should first log and the System.err -- we need make sure thins get printed even if logging not configured proeprly. We don't need to avoid JVM crash, since that class calls exit(1) anyway.