Skip to content

Update airbase to version 124 #11900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nineinchnick wants to merge 3 commits into from
Closed

Update airbase to version 124 #11900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6a0099
Update airbase to version 124
Apr 11, 2022
e16a7af
Restore default order of test execution
Apr 13, 2022
7bb9d77
Avoid printing to standard output in tests
Apr 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions core/trino-main/src/test/java/io/trino/execution/TestingPageSourceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@
public class TestingPageSourceProvider
implements ConnectorPageSourceProvider
{
public TestingPageSourceProvider()
{
System.out.println();
Copy link
Copy Markdown
Member

@findepi findepi Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

anyway, crashing whenever we have System.out isn't practcal....

Copy link
Copy Markdown
Member Author

@nineinchnick nineinchnick Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we probably won't do this update, but I wanted to check if that'll work and are there going to be any gainst in job times.

I don't want to start bikeshedding, but are there any best practices about tests using logging or printing directly?

Copy link
Copy Markdown
Member

@findepi findepi Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i usually prefer logging to direct sout

Copy link
Copy Markdown
Member

@hashhar hashhar Apr 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that Surefire itself doesn't like sout since it uses that for passing control messages and crashes depending on when a sout is done.

Newer versions have this fixed though it seems but required some config - https://stackoverflow.com/a/61936537/4594699

Copy link
Copy Markdown
Member

@findepi findepi Apr 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newer versions have this fixed though it seems but required some config - https://stackoverflow.com/a/61936537/4594699

for posterity

  • this references https://issues.apache.org/jira/browse/SUREFIRE-1614
  • and suggests to use 
    <plugin>
   <artifactId>maven-surefire-plugin</artifactId>
   <version>3.0.0-M5</version>
   <configuration>
     <!-- Activate the use of TCP to transmit events to the plugin -->
     <forkNode implementation="org.apache.maven.plugin.surefire.extensions.SurefireForkNodeFactory"/>
   </configuration>
 </plugin>

Copy link
Copy Markdown
Member Author

@nineinchnick nineinchnick Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a comment in the Surefire Jira issue that it is supposed to be fixed on master, so I guess we can wait for the next release. I'll close this PR until then.

Copy link
Copy Markdown
Member

@findepi findepi May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow-up: downgrade surefire in airbase: airlift/airbase#313

}

@Override
public ConnectorPageSource createPageSource(
ConnectorTransactionHandle transaction,
Expand Down
7 changes: 0 additions & 7 deletions plugin/trino-delta-lake/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@
<properties>
<air.main.basedir>${project.parent.basedir}</air.main.basedir>

<!--
Project's default for air.test.parallel is 'methods'. By design, 'classes' makes TestNG run tests from one class in a single thread.
As a side effect, it prevents TestNG from initializing multiple test instances upfront, which happens with 'methods'.
A potential downside can be long tail single-threaded execution of a single long test class.
TODO (https://github.com/trinodb/trino/issues/11294) remove when we upgrade to surefire with https://issues.apache.org/jira/browse/SUREFIRE-1967
-->
<air.test.parallel>classes</air.test.parallel>
<!-- TODO (https://github.com/trinodb/trino/issues/11325): enable parallel tests; disabled due to tests crashing -->
<air.test.thread-count>1</air.test.thread-count>
</properties>
Expand Down
8 changes: 0 additions & 8 deletions plugin/trino-sqlserver/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@

<properties>
<air.main.basedir>${project.parent.basedir}</air.main.basedir>

<!--
Project's default for air.test.parallel is 'methods'. By design, 'classes' makes TestNG run tests from one class in a single thread.
As a side effect, it prevents TestNG from initializing multiple test instances upfront, which happens with 'methods'.
A potential downside can be long tail single-threaded execution of a single long test class.
TODO (https://github.com/trinodb/trino/issues/11294) remove when we upgrade to surefire with https://issues.apache.org/jira/browse/SUREFIRE-1967
-->
<air.test.parallel>classes</air.test.parallel>
</properties>

<dependencies>
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>io.airlift</groupId>
<artifactId>airbase</artifactId>
<version>123</version>
<version>124</version>
</parent>

<groupId>io.trino</groupId>
Expand Down