Skip to content

Don't silently ignore failures in RPM test step in CI#14257

Merged
findepi merged 1 commit intotrinodb:masterfrom
nineinchnick:fix-rpm-test
Sep 23, 2022
Merged

Don't silently ignore failures in RPM test step in CI#14257
findepi merged 1 commit intotrinodb:masterfrom
nineinchnick:fix-rpm-test

Conversation

@nineinchnick
Copy link
Copy Markdown
Member

Description

I just noticed the RPM test step is failing silently in the CI. I removed the code that ignores errors and also fixed the cause of failure (missing dependencies).

Because the RPM test is a separate step, it requires other modules build in this job to be installed in the local repo, to avoid building most modules twice. This, in turn, requires manually cleaning them later to avoid caching them in Github, because only dependencies should be cached.

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:

Because the RPM test is a separate step, it requires other modules build
in this job to be installed in the local repo, to avoid building most
modules twice. This in turn requires to manually clean them later
to avoid caching them in Github, because only dependencies should be
cached.
@cla-bot cla-bot Bot added the cla-signed label Sep 22, 2022
@nineinchnick nineinchnick requested review from ebyhr, findepi and hashhar and removed request for ebyhr September 22, 2022 15:53
@nineinchnick
Copy link
Copy Markdown
Member Author

nineinchnick commented Sep 22, 2022

I forgot to mention in the commit message that I reverted b2888b4. Should I mention this? It did not revert cleanly anyway.

Comment thread .github/workflows/ci.yml
- name: Test Server RPM
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
$RETRY bash -c "$MAVEN verify -B --strict-checksums -P ci -pl :trino-server-rpm || find core/trino-server-rpm/ -exec ls -ald {} +"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sorry for that

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's me who should be sorry, I broke this in #13367. I should have come up with a better PR title here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think #7547 is at fault

@findepi findepi merged commit 3efa444 into trinodb:master Sep 23, 2022
@github-actions github-actions Bot added this to the 398 milestone Sep 23, 2022
@nineinchnick nineinchnick deleted the fix-rpm-test branch September 23, 2022 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants