Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workflows/tests: use container #923

Merged
merged 3 commits into from
May 26, 2023

Conversation

carlocab
Copy link
Member

This should help simplify this workflow a bit.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Seems much better (if you can get it working)!

@carlocab
Copy link
Member Author

if you can get it working

Not optimistic 🙃

@Bo98
Copy link
Member

Bo98 commented May 17, 2023

setup-ruby is not supported in containers. The expectation is you install it yourself.

The reason being that containers won't necessarily match the host OS, and the toolcache is from the host OS.

@Bo98
Copy link
Member

Bo98 commented May 17, 2023

I think the easiest solution is to split the rspec stuff into their own job without a container. No bundle install should be needed for regular command testing - it's only for rspec.

@MikeMcQuaid
Copy link
Member

I think the easiest solution is to split the rspec stuff into their own job without a container. No bundle install should be needed for regular command testing - it's only for rspec.

Agreed 👍🏻

- uses: actions/checkout@main

- name: Set up Ruby
if: runner.os == 'Linux'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if: runner.os == 'Linux'

as this only runs on Linux

Copy link
Member Author

Choose a reason for hiding this comment

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

Not anymore! I thought it would be less confusing if all RSpec tests ran inside only the rspec job, instead of a weird Linux-macOS split. But I can change it back to making the rspec job Linux-only if desired.

@carlocab carlocab force-pushed the github-container branch 2 times, most recently from 3c1776a to 87bf309 Compare May 25, 2023 13:21
This should help simplify this workflow a bit.
@carlocab carlocab force-pushed the github-container branch 2 times, most recently from 2942999 to 6c842fd Compare May 25, 2023 13:35
We need `setup-ruby` to do this, but this isn't supported inside
containers. As a workaround, let's run the RSpec tests in a separate job
that does not use a container.
@carlocab
Copy link
Member Author

Seems to be erroring at

FileUtils.ln @bottle_filename, download_strategy.cached_location, force: true

on Linux. Not entirely clear on why...

@MikeMcQuaid
Copy link
Member

@carlocab Need to run it from another directory so it's not trying to create a symlink across multiple devices/mount points. mount -a will probably help here.

This is needed to avoid

    Error: Invalid cross-device link @ rb_file_s_link - (./testbottest--0.1.x86_64_linux.bottle.1.tar.gz, /github/home/.cache/Homebrew/downloads/9688e5b8937a63085d076f920b62cbb0139525ccca316ad61fb81e9e314a9382--testbottest--0.1.x86_64_linux.bottle.1.tar.gz)
@carlocab carlocab marked this pull request as ready for review May 26, 2023 08:12
@carlocab
Copy link
Member Author

@carlocab Need to run it from another directory so it's not trying to create a symlink across multiple devices/mount points. mount -a will probably help here.

Thanks; fixed now. Required tests need updating though.

@MikeMcQuaid MikeMcQuaid merged commit fbb432f into Homebrew:master May 26, 2023
4 checks passed
@MikeMcQuaid
Copy link
Member

Thanks again @carlocab!

@carlocab carlocab deleted the github-container branch May 26, 2023 08:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants