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

Add support for Alpine (Linux Distro) #331

Merged
merged 34 commits into from
Sep 22, 2024

Conversation

skinner-m-c
Copy link
Contributor

@skinner-m-c skinner-m-c commented Sep 16, 2024

📚 Description

Add automation for running Bashunit tests in Alpine locally and with Github actions.

🔖 Changes

Adds a target in the Makefile for running tests in Alpine. There are other approaches that could be done (e.g. entirely in Github actions, but that would make contributors unable to test changes in Alpine locally without additional work).

This change does not include fixes for running tests in Alpine. I do have some fixes that I was going to do in a separate pull request because they are a bit involved.

There are several ways to implement this kind of testing. Let me know if there is a preferred approach.

This automation does fail because bashunit does not currently pass in Alpine. Because this change will cause builds to fail, I recommend commenting or excluding out the Github action changes for running these tests and uncomment or add them back when the Alpine test pass. I have included the Github action for sake of completion.

✅ To-do list

  • I updated the CHANGELOG.md to reflect the new feature or fix

CHANGELOG.md Outdated Show resolved Hide resolved
@skinner-m-c skinner-m-c force-pushed the fix/add-alpine-test-automation branch 2 times, most recently from b3f1fb6 to 8661836 Compare September 17, 2024 02:56
Makefile Outdated Show resolved Hide resolved
@skinner-m-c skinner-m-c force-pushed the fix/add-alpine-test-automation branch 2 times, most recently from 930bc47 to 306c935 Compare September 18, 2024 00:40
@skinner-m-c
Copy link
Contributor Author

Thanks for approving the test runs. It has now reached the first error for Alpine that I intend to fix in other pull request. Let me know what you think should be done next. I can remove the Github action and re-add it at a later time when Alpine test pass.

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
@Chemaclass
Copy link
Member

Chemaclass commented Sep 21, 2024

Update: I managed to set up the docker image locally

docker run --rm -it -v "$(pwd)":/project -w /project alpine:latest \
    sh -c  "apk add bash make shellcheck git curl perl && bash"
# and then running `./bashunit` internally
Screenshot 2024-09-22 at 14 17 56

@Chemaclass Chemaclass force-pushed the fix/add-alpine-test-automation branch 2 times, most recently from 3f0270d to 9087bef Compare September 21, 2024 20:19
@Chemaclass Chemaclass changed the title fix: add test automation for Alpine Add support for Alpine (Linux Distro) Sep 21, 2024
@Chemaclass
Copy link
Member

Chemaclass commented Sep 21, 2024

@skinner-m-c, please check the changes and let me know your thoughts. I plan to merge it today :)

@Chemaclass Chemaclass force-pushed the fix/add-alpine-test-automation branch 2 times, most recently from 14bc969 to 6aefdd5 Compare September 22, 2024 12:34
if [[ "$_OS" == "Windows" ]]; then
if [[ "$_OS" == "Windows" || $_DISTRO = "Alpine" ]]; then
Copy link
Member

@Chemaclass Chemaclass Sep 22, 2024

Choose a reason for hiding this comment

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

I got these tests/unit/directory_test.sh failing on Alpine for some reason (directory permissions), but I don't know how to solve them now, so I'd better skip them for now, and work on them in another iteration/PR

# Running these tests without ` || $_DISTRO = "Alpine"`:
docker run --rm -it -v "$(pwd)":/project -w /project alpine:latest \
    sh -c  "apk add bash make shellcheck git curl perl && ./bashunit tests/unit/directory_test.sh"
Screenshot 2024-09-22 at 14 57 09

@Chemaclass Chemaclass added the enhancement New feature or request label Sep 22, 2024
@Chemaclass Chemaclass self-assigned this Sep 22, 2024
@Chemaclass Chemaclass merged commit 938b8df into TypedDevs:main Sep 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants