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

Matrix builds for tests #29

Merged
merged 5 commits into from
Oct 21, 2022
Merged

Conversation

AlexNDRmac
Copy link
Contributor

Hi @sp1thas

Added:

  • Matrix for unit test job

Changed:

  • Added workflow yaml files to paths-ignore in testing.yml to avoid run build when it does not make sense
  • Updated actions/checkout@v2 to actions/checkout@v3 because old version has deprecated node12

Closes #28

Notes:

macOS tests will fail if you does't have GNU diff and GNU grep (diffutils and grep in brew) in your system.

Also, I found possible bug in dropboxignore.sh which I would recommend to resolve with additional check:

[[ $MACHINE == Darwin ]] && GREP_CMD="ggrep" || GREP_CMD="grep"
[[ $MACHINE == Darwin ]] && DIFF_CMD="$(brew --prefix)/bin/diff" || DIFF_CMD="diff"

If you does't have ggrep and diff in brew path – you'll see error which is not related to dropbox ignore features, it's error with third party software which you use inside script. It'll be more explicit if you'll check these tools before usage. With macOS default grep - tests also fails with error /usr/local/bin/diff: unrecognized option --ignore-trailing-space'`

For macOS users - I would recommend you to add all these prerequisites to Readme or other documentation.

@AlexNDRmac
Copy link
Contributor Author

@sp1thas
Copy link
Owner

sp1thas commented Oct 20, 2022

Thanks @AlexNDRmac for this PR, your contribution was really helpful. Given that I'm not a macOS user, it's a bit tricky to test it on my own. The necessary packages are mentioned in the readme file and also, and the packages are installed using the kickstart script

Regarding the bug, the check you proposed is exactly the same as this one. Did I miss something?

.github/workflows/testing.yml Show resolved Hide resolved
.github/workflows/testing.yml Show resolved Hide resolved
@AlexNDRmac
Copy link
Contributor Author

I missed your kickstarter script :) it solves question with dependencies.
About additional checks for diff and grep - i told about more strict checks, smth like:

if [ "$(command -v ggper 2>/dev/null)" = "" ]; then
  (>&1 echo "Required tool 'ggrep' not found...")
  (>&1 echo "Install it with 'brew install grep'")
  exit 1
fi

Copy link
Owner

@sp1thas sp1thas left a comment

Choose a reason for hiding this comment

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

LGTM!

@sp1thas sp1thas merged commit 639eb34 into sp1thas:master Oct 21, 2022
@AlexNDRmac AlexNDRmac deleted the matrix-biulds-for-tests branch October 21, 2022 11:06
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.

Run tests using multiple github runners
2 participants