Skip to content

Hyrax-2190: Upload test logs for Docker#1398

Merged
kneumiller merged 34 commits into
masterfrom
travis-test-logs
Jul 2, 2026
Merged

Hyrax-2190: Upload test logs for Docker#1398
kneumiller merged 34 commits into
masterfrom
travis-test-logs

Conversation

@kneumiller

@kneumiller kneumiller commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

Reference ticket: HYRAX-2190

TODO

Tasks

  • Ticket exists and is linked in title
  • Tests added/updated
  • Dead code removed
  • No TODOs added

@jgallagher59701 jgallagher59701 left a comment

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.

Look at these changes - see if the simpler docker run .... -c "..." works and if so lets use that. Else, put a comment in the Dockerfile explaining what's going on. Thanks.

Comment thread .travis.yml
export SNAPSHOT_IMAGE_TAG="opendap/${DOCKER_NAME}:snapshot-${DIST}"
export BES_REPO_DIR=${TRAVIS_BUILD_DIR}
- ./travis/build-rhel-docker.sh
# Note: Take the test log tarball that was created in the Dockerfile and copied to the final mount

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.

Maybe: and copied to the --> and copy it to the

Comment thread .travis.yml Outdated
- ./travis/build-rhel-docker.sh
# Note: Take the test log tarball that was created in the Dockerfile and copied to the final mount
# and then run this copy command to copy it into Travis.
# Use "$1" because Single quotes prevent the host shell (Travis) from expanding ${TRAVIS_JOB_NUMBER} before invoking

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 had to ask Claude about this pattern - the -c 'cp ...' sh "/scratch..." part.

It becomes: cp /bes-test-logs/bes-test-logs.tar.gz /scratch/bes-autotest-${TRAVIS_JOB_NUMBER}-logs.tar.gz.

The reason for the obscure $1 and `sh "/scratch/...${TRAVIS_JOB_NUMBER}..." is that the TRAVIS_JOB_NUMBER env var gets expanded in the container using the pattern. I'll forget that by next week ;-) We better put in a comment like: This pattern means the environment variable is passed into the container where it is then expanded.

But, now that I've thought about it, I don't think we need to do that. Since the TRAVIS_JOB_NUMBER is set in/by Travis, it'll have the same value in both places. I think you can use the plain:

-c "cp /bes-test-logs/bes-test-logs.tar.gz /scratch/bes-autotest-${TRAVIS_JOB_NUMBER}-logs.tar.gz"

and it'll work (and we will know what's going on and won't have to write a huge comment. ;-)

@kneumiller kneumiller requested a review from ndp-opendap June 29, 2026 15:14

@ndp-opendap ndp-opendap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good.

Have you confirmed that the test log tar ball is not included in the end product bes_core image generated by Travis and used downstream?

@ndp-opendap ndp-opendap self-requested a review July 2, 2026 15:25

@ndp-opendap ndp-opendap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm satisfied and we can move on.

@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

@kneumiller kneumiller merged commit 3d34864 into master Jul 2, 2026
7 of 8 checks passed
@kneumiller kneumiller deleted the travis-test-logs branch July 2, 2026 17:02
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