Skip to content

Conversation

@majetideepak
Copy link
Collaborator

@majetideepak majetideepak commented Jun 8, 2023

#19722 split the tests to run in parallel. After this change not all tests seem to be running.
CircleCI linux-build with this change shows
[INFO] Tests run: 26 for presto-native
[INFO] Tests run: 18 for spark-native
https://app.circleci.com/pipelines/github/prestodb/presto/3257/workflows/07c55503-4fa9-4f3f-843c-0d479d47af81/jobs/5000

The previous count was
[INFO] Tests run: 162 for presto-native
[INFO] Tests run: 101 for presto-spark
https://app.circleci.com/pipelines/github/prestodb/presto/3203/workflows/ce3eda3f-95ab-4035-b8b2-c52788a2c23c/jobs/4811

The timings also do not match.

This reverts commit 446bde5.

Resolves #19816

== NO RELEASE NOTE ==

CircleCI linux-build with this change shows
[INFO] Tests run: 26 for presto-native
[INFO] Tests run: 18 for spark-native
https://app.circleci.com/pipelines/github/prestodb/presto/3257/workflows/07c55503-4fa9-4f3f-843c-0d479d47af81/jobs/5000

The previous count was
[INFO] Tests run: 257
https://app.circleci.com/pipelines/github/prestodb/presto/3124/workflows/303c2b53-a04d-4bb9-bf02-e202a891521a/jobs/4539

The timings also do not match.

This reverts commit 446bde5.
@majetideepak majetideepak requested a review from a team as a code owner June 8, 2023 13:47
@majetideepak
Copy link
Collaborator Author

@mshang816 Can you please take a look at this issue? Thanks.

@kgpai
Copy link
Contributor

kgpai commented Jun 9, 2023

cc: @ajaygeorge

@majetideepak
Copy link
Collaborator Author

The counts are now back to the original
https://app.circleci.com/pipelines/github/prestodb/presto/3550/workflows/a53f8687-68c9-4393-84db-08c34a32c1a9/jobs/6046

@mbasmanova, @ajaygeorge can one of you please help approve and merge this? This is a priority as not all Prestissimo E2E tests are running on master.

@mbasmanova mbasmanova merged commit 4e6db32 into prestodb:master Jun 9, 2023
@mshang816
Copy link
Contributor

mshang816 commented Jun 9, 2023

@majetideepak that's because there are 5 jobs run in parallel but it looks like you only count one job.
image

So for each run:

  • run 0: 26 /18 (presto/spark)
  • run 1: 6/6 (presto/spark)
  • run 2: 22/22 (presto/spark)
  • run 3: 62/46 (presto/spark)
  • run 4: 46/9 (presto/spark)

Then totally: 162/101 (presto/spark) so the numbers do add up correctly. It's just you missed tabs for other 4 parallel jobs.
And we did check the results before merging the PR.
If it's ok, let's bring the change back as it greatly cut our e2e execution in half.
c.c. @mbasmanova

@mshang816
Copy link
Contributor

The counts are now back to the original https://app.circleci.com/pipelines/github/prestodb/presto/3550/workflows/a53f8687-68c9-4393-84db-08c34a32c1a9/jobs/6046

@mbasmanova, @ajaygeorge can one of you please help approve and merge this? This is a priority as not all Prestissimo E2E tests are running on master.

ALL E2E tests are run actually, see my explanation below.

@majetideepak
Copy link
Collaborator Author

@mshang816 apologies for misunderstanding the output. I will open a PR to add this back. I did not see the other runs as you guessed. Probably we can print something like run 1/5 in the console output as well.

@majetideepak
Copy link
Collaborator Author

@mshang816 the PR #19859 adds this back. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[native] Not all E2E tests seem to be running

4 participants