Extract some product tests into a separate suites #14818

nineinchnick · 2022-10-28T13:23:58Z

Description

Suite 6 and 7 total run time is nearly 1 hour so split them up. Check the running times at the end of the product tests step at https://github.com/trinodb/trino/actions/runs/3343271418/jobs/5537463265

Non-technical explanation

n/a

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

...-tests-launcher/src/main/java/io/trino/tests/product/launcher/suite/suites/SuiteClients.java

nineinchnick · 2022-11-07T08:42:26Z

This branch:

master:

We can see that suite 6 and 7 improved over 20 minutes, getting them down to about 30 minutes.

Suite 1 and 2 got improved only slightly, by ~10 minutes, getting them just below an hour. If this gets approved, we can continue splitting more tests out of these suites, but I don't want to add more commits in this PR.

MiguelWeezardo · 2022-11-07T10:45:08Z

This branch:

master:

We can see that suite 6 and 7 improved over 20 minutes, getting them down to about 30 minutes.

Suite 1 and 2 got improved only slightly, by ~10 minutes, getting them just below an hour. If this gets approved, we can continue splitting more tests out of these suites, but I don't want to add more commits in this PR.

I wonder why both branches have 24 PT job count. Shouldn't there be more jobs executed on this branch now that new suites have been created?

nineinchnick · 2022-11-07T10:47:10Z

I wonder why both branches have 24 PT job count. Shouldn't there be more jobs executed on this branch now that new suites have been created?

This is a coincidence, master runs additional jobs with secrets, like Azure and GCP tests.

nineinchnick · 2022-11-10T09:21:48Z

@hashhar PTAL

hashhar · 2022-11-10T10:28:04Z

What is the affect on wall-time?

cc: @findepi regarding the direction (not actual changes) since I know you have opinions on this.

nineinchnick · 2022-11-10T11:26:20Z

What is the affect on wall-time?

The overhead of a single PT job is about 50s, if you sum all steps except Product Tests:

This PR adds 9 new jobs. So I guess this adds ~10 minutes. But I don't have any exact statistics what's the total run time for all PT jobs and which ones we run most often since we don't run all of them in every PR.

Maybe this will make it easier to find failures in PTs if the test suites are smaller.

nineinchnick · 2022-11-10T11:29:30Z

BTW this change was requested by @electrum

MiguelWeezardo · 2022-11-10T14:50:20Z

This might also let us avoid running suite6 and suite7 for PRs with only those plugin changes.

nineinchnick · 2022-11-14T12:20:45Z

@findepi PTAL

...ests-launcher/src/main/java/io/trino/tests/product/launcher/suite/suites/SuiteFunctions.java

findepi · 2022-11-14T13:04:43Z

...uct-tests-launcher/src/main/java/io/trino/tests/product/launcher/suite/suites/SuiteTpch.java

+    {
+        return ImmutableList.of(
+                testOnEnvironment(EnvMultinode.class)
+                        .withGroups("configured_features", "tpch")


when do i write "configured_features" ?

When you want to ensure that the test environment (EnvMultinode.class) properly defines all configured features and doesn't have any additional ones (like extra catalogs). If that would happen, if there are tests that rely on those features, they could be skipped in some PRs.

Since we reuse the same environments in multiple suites if some don't run tests from the configured_features group, nothing terrible would happen, but it's good to keep it in every suite for consistency. I hope that's why you noticed it's missing.

.../src/main/java/io/trino/tests/product/launcher/suite/suites/SuiteStorageFormatsDetailed.java

findepi · 2022-11-14T13:07:28Z

cc: @findepi regarding the direction (not actual changes) since I know you have opinions on this.

I like the direction

hashhar

LGTM % can we verify we run same number of tests?

In past I've noticed such refactor end up running same test group as part of multiple suites (which is ok but reverse is also possible - some group not being run at all).

We probably write surefire reports for PTs which would include test counts (with fully qualified names as well) so we can disable impact analysis, do a run before this change, another with this and see what we see?

nineinchnick · 2022-11-15T08:27:35Z

We don't have functions to parse XML in Trino, but I used regexes to extract test class names from test reports stored as GitHub run artifacts:

I compared these runs:

this branch: https://github.com/trinodb/trino/actions/runs/3461805232
master (last green one): https://github.com/trinodb/trino/actions/runs/3457214593

with classes as (
  select run_id, array_agg(distinct test_class order by test_class) as classes
  from artifacts
  cross join unnest(regexp_extract_all(from_utf8(contents), 'class name="(.*)"', 1)) as c(test_class)
  where run_id in (3461805232, 3457214593) and name like 'test report pt %'
  group by run_id
)
select array_join(array_except(
  (select classes from classes where run_id = 3461805232),
  (select classes from classes where run_id = 3457214593)), U&'\000A') as missing;

which gives:

                        missing                        
-------------------------------------------------------
 io.trino.tempto.array_functions                       
 io.trino.tempto.binary_functions                      
 io.trino.tempto.hive_tpch                             
 io.trino.tempto.horology_functions                    
 io.trino.tempto.json_functions                        
 io.trino.tempto.map_functions                         
 io.trino.tempto.math_functions                        
 io.trino.tempto.regex_functions                       
 io.trino.tempto.string_functions                      
 io.trino.tempto.url_functions                         
 io.trino.tests.product.TestFunctions                  
 io.trino.tests.product.TestImpersonation              
 io.trino.tests.product.teradata.TestTeradataFunctions 
(1 row)

Most of these look like they should be in suite-functions, I'm looking into it.

nineinchnick · 2022-11-15T08:53:53Z

Whoops, I reversed the ids in array_except(). So it looks like the tests in my previous comment are the ones we're not running on master right now.

The tests I'm missing in this branch are:

                          missing                          
-----------------------------------------------------------
 io.trino.tests.product.deltalake.TestDeltaLakeGcs         
 io.trino.tests.product.hive.TestAbfsSyncPartitionMetadata 
(1 row)

nineinchnick · 2022-11-15T08:56:31Z

And the ones above require secrets, so it makes sense I'm not running them in my fork.

nineinchnick · 2022-11-15T10:49:32Z

False alarm, I had a bug in the connector, it was skipping zip file entries when the size was unknown. I made sure I'm getting all artifacts and reports and tests missing here are:

                                         missing                                          
------------------------------------------------------------------------------------------
 io.trino.tests.product.deltalake.TestDatabricksWithGlueMetastoreCleanUp                  
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCheckpointsCompatibility         
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCompatibilityCleanUp             
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCreateTableAsSelectCompatibility 
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCreateTableCompatibility         
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksPartitioningCompatibility        
 io.trino.tests.product.deltalake.TestDeltaLakeGcs                                        
 io.trino.tests.product.deltalake.TestDeltaLakeWriteDatabricksCompatibility               
 io.trino.tests.product.hive.TestAbfsSyncPartitionMetadata                                
 io.trino.tests.product.iceberg.TestIcebergOptimize                                       
 io.trino.tests.product.iceberg.TestIcebergPartitionEvolution                             
 io.trino.tests.product.iceberg.TestIcebergProcedureCalls                                 
 io.trino.tests.product.iceberg.TestIcebergSparkCompatibility                             
 io.trino.tests.product.iceberg.TestIcebergSparkDropTableCompatibility                    
(1 row)

Suite 6 total run time is near 1 hour so split it up

Suite 7 total run time is near 1 hour so split it up

Suite 1 total run time is over 1 hour so split it up

Suite 2 total run time is over 1 hour so split it up

nineinchnick · 2022-11-15T10:58:07Z

Iceberg tests are missing because the job failed. I think I saw it green before, so I rebased and I'm running the CI again.

nineinchnick · 2022-11-16T09:42:28Z

I compared it again with https://github.com/trinodb/trino/actions/runs/3470415629 and there are no extra tests and we're missing only ones that require secrets:

                                         missing                                          
------------------------------------------------------------------------------------------
 io.trino.tests.product.deltalake.TestDatabricksWithGlueMetastoreCleanUp                  
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCheckpointsCompatibility         
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCompatibilityCleanUp             
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCreateTableAsSelectCompatibility 
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksCreateTableCompatibility         
 io.trino.tests.product.deltalake.TestDeltaLakeDatabricksPartitioningCompatibility        
 io.trino.tests.product.deltalake.TestDeltaLakeGcs                                        
 io.trino.tests.product.deltalake.TestDeltaLakeWriteDatabricksCompatibility               
 io.trino.tests.product.hive.TestAbfsSyncPartitionMetadata                                
(1 row)

cla-bot bot added the cla-signed label Oct 28, 2022

nineinchnick changed the title ~~Extract Kafka product tests into a separate suite~~ Extract some product tests into a separate suites Oct 28, 2022

nineinchnick force-pushed the extract-kafka-pt-suite branch 3 times, most recently from 6666351 to 85fe883 Compare November 2, 2022 15:48

MiguelWeezardo reviewed Nov 2, 2022

View reviewed changes

...-tests-launcher/src/main/java/io/trino/tests/product/launcher/suite/suites/SuiteClients.java Outdated Show resolved Hide resolved

nineinchnick force-pushed the extract-kafka-pt-suite branch from 85fe883 to c622af0 Compare November 3, 2022 08:31

nineinchnick requested a review from hashhar November 4, 2022 08:08

MiguelWeezardo self-requested a review November 7, 2022 10:51

MiguelWeezardo approved these changes Nov 7, 2022

View reviewed changes

nineinchnick force-pushed the extract-kafka-pt-suite branch from c622af0 to 8168180 Compare November 10, 2022 09:22

hashhar requested review from findepi and removed request for hashhar November 10, 2022 10:28

findepi reviewed Nov 14, 2022

View reviewed changes

nineinchnick force-pushed the extract-kafka-pt-suite branch from 8168180 to 0facaf3 Compare November 14, 2022 13:18

hashhar reviewed Nov 14, 2022

View reviewed changes

nineinchnick added 7 commits November 15, 2022 11:57

Extract Kafka product tests into a separate suite

c68c25b

Suite 6 total run time is near 1 hour so split it up

Extract Cassandra product tests into a separate suite

77c0044

Suite 6 total run time is near 1 hour so split it up

Extract Clickhouse product tests into a separate suite

21a85ec

Suite 6 total run time is near 1 hour so split it up

Extract MySQL product tests into a separate suite

4343fef

Suite 7 total run time is near 1 hour so split it up

Extract Iceberg product tests into a separate suite

a080b72

Suite 7 total run time is near 1 hour so split it up

Split up product tests suite 1

69ccde0

Suite 1 total run time is over 1 hour so split it up

Split up product tests suite 2

ed69894

Suite 2 total run time is over 1 hour so split it up

nineinchnick force-pushed the extract-kafka-pt-suite branch from 0facaf3 to ed69894 Compare November 15, 2022 10:57

hashhar approved these changes Nov 16, 2022

View reviewed changes

hashhar merged commit cc764ed into trinodb:master Nov 16, 2022

github-actions bot added this to the 404 milestone Nov 16, 2022

nineinchnick deleted the extract-kafka-pt-suite branch November 17, 2022 08:12

nineinchnick mentioned this pull request Nov 18, 2022

Some product tests are executed in multiple test suites #15096

Closed

colebow mentioned this pull request Nov 21, 2022

Add Trino 405 release notes #15139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract some product tests into a separate suites #14818

Extract some product tests into a separate suites #14818

nineinchnick commented Oct 28, 2022 •

edited

Loading

nineinchnick commented Nov 7, 2022 •

edited

Loading

MiguelWeezardo commented Nov 7, 2022

nineinchnick commented Nov 7, 2022

nineinchnick commented Nov 10, 2022

hashhar commented Nov 10, 2022 •

edited

Loading

nineinchnick commented Nov 10, 2022

nineinchnick commented Nov 10, 2022

MiguelWeezardo commented Nov 10, 2022

nineinchnick commented Nov 14, 2022

findepi Nov 14, 2022

nineinchnick Nov 14, 2022

findepi commented Nov 14, 2022

hashhar left a comment •

edited

Loading

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 16, 2022

Extract some product tests into a separate suites #14818

Extract some product tests into a separate suites #14818

Conversation

nineinchnick commented Oct 28, 2022 • edited Loading

Description

Non-technical explanation

Release notes

nineinchnick commented Nov 7, 2022 • edited Loading

MiguelWeezardo commented Nov 7, 2022

nineinchnick commented Nov 7, 2022

nineinchnick commented Nov 10, 2022

hashhar commented Nov 10, 2022 • edited Loading

nineinchnick commented Nov 10, 2022

nineinchnick commented Nov 10, 2022

MiguelWeezardo commented Nov 10, 2022

nineinchnick commented Nov 14, 2022

findepi Nov 14, 2022

Choose a reason for hiding this comment

nineinchnick Nov 14, 2022

Choose a reason for hiding this comment

findepi commented Nov 14, 2022

hashhar left a comment • edited Loading

Choose a reason for hiding this comment

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 15, 2022

nineinchnick commented Nov 16, 2022

nineinchnick commented Oct 28, 2022 •

edited

Loading

nineinchnick commented Nov 7, 2022 •

edited

Loading

hashhar commented Nov 10, 2022 •

edited

Loading

hashhar left a comment •

edited

Loading