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

Refactor to make scripts POSIX compliant #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mondeja
Copy link

@mondeja mondeja commented Jan 11, 2022

Hi,

This pull request is a proposal for a refactoring of test files to use Bourne Shell POSIX compliant features and syntax. The main benefits of this change is that tests can be executed with other POSIX compliant shells, not only with Bash. This is especially useful in certain contexts where the system doesn't include Bash installed by default or to increase tests execution performance in pipelines using other shells.

Implementation details

  • Shebangs have been fixed because the test files were being run with bash but their shebangs were pointing to /bin/sh (Bourne Shell). Changed to /usr/bin/env sh to give more flexibility about which sh implementation will use (see "POSIX shell scripts shebang #!/bin/sh vs #!/usr/bin/env sh, any difference?" in StackExchange). This allow to execute the tests directly with ./path/to/file.sh and the underlying system can determine which shell should use.
  • Local scoped variable names have been prepended with the name of the function that scopes them to avoid colissions between variable names. This decreases readability of the code but is strict POSIX compliant and required to make it work in Solaris SunOS and other strange systems. Let me know if other practice is preferred instead.
  • Common helper functions are now reused between test scripts.

@mondeja mondeja changed the title Refactor tests to make scripts POSIX compliant Refactor to make scripts POSIX compliant Jan 11, 2022
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.

1 participant