Feature/add test for installer permissions #56
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a bats @test to prove that install.sh only fixes the permissions of installed files.
This reverts commit ab52948. The provided test failed on AppVeyor and Travis. This is very likely a problem of the test implementation itself. AppVeyor runs on a windows system. So the output of `stat -c '%A'` will very likely be different than for a linux system. AppVeyor error message: ``` not ok 44 installer only fixes file permissions of installed files ``` Travis fails on alpine linux while calling the install script: ``` not ok 44 installer only fixes file permissions of installed files ``` The problems need to be researched and fixed before adding this test again.
The test makes sure that install.sh doesn't change the permissions of files other than the installed ones. After applying this commit, make sure to rebuild your docker image: ``` docker build --tag bats:latest . ``` The change to the docker file was necessary for the Travis CI test to succeed. It also performs a test using the docker image and it wasn't able to find install.sh in that case. However, because we're running tests in different environment, it was inevitable to refer to install.sh using a relative path. Testing with the git bash in AppVeyor was another challenge. The execution flags of installed and previously existing files needed to be checked. For crossplatform scenarios `find` is the easiest solution for checking the execution permissions. However, the git bash – on a Windows system, as performed by the AppVeyor test – would use Windows `find`, which expects a different parameter syntax compared to gnu find. This can be solved by using `/bin/find` in the git bash. See https://stackoverflow.com/questions/21438556/finding-files-in-a-git-bash-terminal#answer-40633578
mbland
added a commit
that referenced
this pull request
Jun 7, 2018
Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves outstanding issues from #32. Per all these earlier issues and PRs: - The original bin/bats symlink was causing problems on Windows. - The attempted fix from #32 created problems for install.sh. - Per #90, changing bin/bats to a one-line script in #88 broke some Bats installations that rely on symlink schemes (such as when installed via https://github.com/basherpm/basher). For an idea why I wanted to abolish using any symlinks if I could for Windows's sake, see the following results I discovered during a search for "git symlink windows": - https://github.com/git-for-windows/git/wiki/Symbolic-Links - https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ - libgit2/libgit2#4107 - https://stackoverflow.com/q/5917249 Using a method inspired by go-core.bash from https://github.com/mbland/go-script-bash, I was able to reliably compute the root of the Bats installation without using `readlink` by making use of `PWD` and `cd`. What's more, in the course of doing this, I realized libexec/bats didn't need `readlink` either, so I removed it, eliminating external dependencies and subshells. I also removed some extraneous variables and small bits of logic. On top of making install.sh, bin/bats, and libexec/bats more self-contained, resilient, and compact, the existing test suite (before the new installer and symlink tests) sped up significantly, running 0.6s-0.7s faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM): Bash 3.2.57(1)-release before: 58 tests, 0 failures real 0m4.924s user 0m3.045s sys 0m1.798s Bash 3.2.57(1)-release after: 58 tests, 0 failures real 0m4.326s user 0m2.793s sys 0m1.508s Bash 4.4.23(1)-release before: 58 tests, 0 failures real 0m5.228s user 0m3.046s sys 0m1.952s Bash 4.4.23(1)-release after: 58 tests, 0 failures real 0m4.563s user 0m2.782s sys 0m1.614s
mbland
added a commit
that referenced
this pull request
Jun 7, 2018
Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves outstanding issues from #32. Per all these earlier issues and PRs: - The original bin/bats symlink was causing problems on Windows. - The attempted fix from #32 created problems for install.sh. - Per #90, changing bin/bats to a one-line script in #88 broke some Bats installations that rely on symlink schemes (such as when installed via https://github.com/basherpm/basher). For an idea why I wanted to abolish using any symlinks if I could for Windows's sake, see the following results I discovered during a search for "git symlink windows": - https://github.com/git-for-windows/git/wiki/Symbolic-Links - https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ - libgit2/libgit2#4107 - https://stackoverflow.com/q/5917249 Using a method inspired by go-core.bash from https://github.com/mbland/go-script-bash, I was able to reliably compute the root of the Bats installation without using `readlink` by making use of `PWD` and `cd`. What's more, in the course of doing this, I realized libexec/bats didn't need `readlink` either, so I removed it, eliminating external dependencies and subshells. I also removed some extraneous variables and small bits of logic. On top of making install.sh, bin/bats, and libexec/bats more self-contained, resilient, and compact, the existing test suite (before the new installer and symlink tests) sped up significantly, running 0.6s-0.7s faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM): Bash 3.2.57(1)-release before: 58 tests, 0 failures real 0m4.924s user 0m3.045s sys 0m1.798s Bash 3.2.57(1)-release after: 58 tests, 0 failures real 0m4.326s user 0m2.793s sys 0m1.508s Bash 4.4.23(1)-release before: 58 tests, 0 failures real 0m5.228s user 0m3.046s sys 0m1.952s Bash 4.4.23(1)-release after: 58 tests, 0 failures real 0m4.563s user 0m2.782s sys 0m1.614s Also tweaks the Dockerfile to update the symlink to point to bin/bats, not libexec/bats.
mbland
added a commit
that referenced
this pull request
Jun 7, 2018
Closes #90. Closes #83. Closes #56. Closes #55. Closes #53. Resolves outstanding issues from #32. Per all these earlier issues and PRs: - The original bin/bats symlink was causing problems on Windows. - The attempted fix from #32 created problems for install.sh. - Per #90, changing bin/bats to a one-line script in #88 broke some Bats installations that rely on symlink schemes (such as when installed via https://github.com/basherpm/basher). For an idea of why I wanted to keep bin/bats as a script due to how symlinks complicate things on Windows, see the following results I discovered during a search for "git symlink windows": - https://github.com/git-for-windows/git/wiki/Symbolic-Links - https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ - libgit2/libgit2#4107 - https://stackoverflow.com/q/5917249 In the course of applying these changes, I realized libexec/bats computed some extraneous variables, so I removed them, eliminating a few external processes and subshells. I also cleaned up other small bits of logic. On top of making install.sh, bin/bats, and libexec/bats more resilient and compact, the existing test suite (before adding the new test/installer.bats file) sped up significantly, running 0.6s-0.7s faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM): Bash 3.2.57(1)-release before: 58 tests, 0 failures real 0m4.924s user 0m3.045s sys 0m1.798s Bash 3.2.57(1)-release after: 58 tests, 0 failures real 0m4.341s user 0m2.808s sys 0m1.540s Bash 4.4.23(1)-release before: 58 tests, 0 failures real 0m5.228s user 0m3.046s sys 0m1.952s Bash 4.4.23(1)-release after: 58 tests, 0 failures real 0m4.582s user 0m2.791s sys 0m1.643s Also tweaks the Dockerfile to update the symlink to point to bin/bats, not libexec/bats.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is meant to be on top of #55. My first attempt to provide a test along with #55 failed, because of cross platform issues and implementation details of the test itself. This is now fixed. As the changes also include an adaption of the Dockerfile , I found it necessary to create a second PR for this.
Add test for installer permissions
The test makes sure that install.sh doesn't change the permissions of files other than the installed ones.
After applying this commit, make sure to rebuild your docker image:
The change to the docker file was necessary for the Travis CI test to succeed. Travis also performs a test using the docker image and it wasn't able to find install.sh in that case. However, because we're running tests in different environments, it was inevitable to refer to install.sh using a relative path.
Testing with the git bash in AppVeyor was another challenge. The execution flags of installed and previously existing files needed to be checked. For cross platform scenarios
findis the easiest solution for checking the execution permissions.However, the git bash – on a Windows system, as performed by the AppVeyor test – would use Windows
find, which expects a different parameter syntax compared to gnu find. This can be solved by using/bin/findin the git bash. See https://stackoverflow.com/questions/21438556/finding-files-in-a-git-bash-terminal#answer-40633578