Skip to content

Conversation

@ostinru
Copy link
Collaborator

@ostinru ostinru commented Dec 22, 2025

This PR consist of two changes:

  1. Update CI pipeline to mark steps failed when we have failed tests.
  2. Save more logs to github artifacts.
  3. Bump testng version. TestNG before 6.9.5 marked all tests as SKIPPED when @BeforeClass methods raised an exception.
  4. Fix Hive tests, by fixing file copy check.

Tests may stuck for some reason and github will kill CI pipeline leaving
us without any clue what happened.
This change limits time for each step to run. When timeout raised -
github will kill single step, and other steps will collect results and
will provide test reports with logs.
@ostinru
Copy link
Collaborator Author

ostinru commented Dec 22, 2025

@tuhaihe , should I sign CLA Agreement? Commit message template says that I should. However there is no description on how this process organized.

@tuhaihe
Copy link
Member

tuhaihe commented Dec 22, 2025

@tuhaihe , should I sign CLA Agreement? Commit message template says that I should. However there is no description on how this process organized.

Hey @ostinru there is no need to sign it under the ASF repos. Sorry for the confusion...

I will update the related files following the ASF policies.

@ostinru ostinru marked this pull request as ready for review December 22, 2025 10:41
FAILED_COUNT="${{ steps.collect_artifacts.outputs.failed_count || 0 }}"
SKIPPED_COUNT="${{ steps.collect_artifacts.outputs.skipped_count || 0 }}"
if [ "${{ steps.run_test.outcome }}" == "failure" ] || [ "$FAILED_COUNT" -gt 0 ] || [ "$SKIPPED_COUNT" -gt 0 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

SKIPPED_COUNT > 0 is failed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sometimes it marks tests as skipped due to unavailability of some jsystem's resources.
Let me check it again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok... It in fact more complicated than I thought before.

Old TestNG versions has bug testng-team/testng#739 - when @BeforeClass(alwaysRun = true) fails - all tests are marked as SKIPPED despite of alwaysRun = true. Fixed in 6.9.5+.

Originally, almost all tests in open-gpdb/pxf failed somwhere inside doInit() methods - and all our tests were marked as SKIPPED. And then we decided to think about such tests as FAILED.

So, it seems we can just bump testng version. In this scenario I will expect that tests will be FAILED instead of SKIPPED. I will update this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks complicated. Please let me know when you're ready for the review. I will review the code.

Old TestNG versions has bug testng-team/testng#739 - when `@BeforeClass(alwaysRun = true)` fails - all tests are marked as SKIPPED despite of `alwaysRun = true`. Fixed in 6.9.5+.
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.

3 participants