Unbreak test suites with multiple files loading common constants #904
Conversation
This file contains 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
debarshiray
added a commit
to debarshiray/bats-core
that referenced
this pull request
Apr 19, 2024
Some editors like GNU Emacs insist on adding the newline at end of file, if it's missing, which leads to noise when looking at the actual code changes. bats-core#904
debarshiray
added a commit
to debarshiray/bats-core
that referenced
this pull request
Apr 19, 2024
Test suites with a helper file with common constants that is loaded by multiple files stopped working with: $ bats /some/path /some/path/test_helper.bash: line 1: A_CONSTANT: readonly variable Error while sourcing library loader at '/some/path/test_helper.bash' If the helper file had anything else other than the common constants, then the tests would run, but the spew about the constants would still remain: $ bats --tap /some/path /some/path/test_helper.bash: line 1: A_CONSTANT: readonly variable 1..2 ok 1 first ok 2 second Fallout from 9d5ecdb bats-core#904
debarshiray
added a commit
to debarshiray/bats-core
that referenced
this pull request
Apr 19, 2024
debarshiray
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
from
b94660b
to
745b619
Compare
|
This goes on top of #902 |
martin-schulze-vireso
requested changes
Apr 20, 2024
There was a problem hiding this comment.
Thanks for your report and suggestion. I believe that we should not ignore load errors during the test registration, that will only lead to hard to diagnose problems.
IMHO, the correct way forward should be to separate the evaluationen of individual test files during test registration in the same way as is done for test execution.
Yeah, I can imagine.
I don't understand the Bats code well enough to know how exactly that should be done. I can try it, if you give me some pointers, or I don't mind if you do it yourself and I hope that the tests would still be useful in that case. :) |
|
Thanks for the review! |
debarshiray
added a commit
to debarshiray/bats-core
that referenced
this pull request
Apr 22, 2024
Test suites with a helper file with common constants that is loaded by multiple files stopped working with: $ bats /some/path /some/path/test_helper.bash: line 1: A_CONSTANT: readonly variable Error while sourcing library loader at '/some/path/test_helper.bash' If the helper file had anything else other than the common constants, then the tests would run, but the spew about the constants would still remain: $ bats --tap /some/path /some/path/test_helper.bash: line 1: A_CONSTANT: readonly variable 1..2 ok 1 first ok 2 second Fallout from 9d5ecdb bats-core#904
debarshiray
added a commit
to debarshiray/bats-core
that referenced
this pull request
Apr 22, 2024
debarshiray
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
from
745b619
to
f94e695
Compare
|
Tried to fix the |
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Apr 30, 2024
Ansible's built-in 'package' module doesn't show any details when installing the RPMs. All that can be seen is: TASK [Install RPM packages] fedora-rawhide | changed Therefore, there's no way to know what version of the packages got installed. In this case, not knowing the Bats version being used by the CI makes it difficult to know why the tests are generating this spew on Fedora Rawhide [1]: TASK [Run system tests] test/system/libs/helpers.bash: line 7: TEMP_BASE_DIR: readonly variable test/system/libs/helpers.bash: line 8: TEMP_STORAGE_DIR: readonly variable test/system/libs/helpers.bash: line 10: IMAGE_CACHE_DIR: readonly variable test/system/libs/helpers.bash: line 11: ROOTLESS_PODMAN_STORE_DIR: readonly variable test/system/libs/helpers.bash: line 12: ROOTLESS_PODMAN_RUNROOT_DIR: readonly variable test/system/libs/helpers.bash: line 13: PODMAN_STORE_CONFIG_FILE: readonly variable test/system/libs/helpers.bash: line 14: DOCKER_REG_ROOT: readonly variable test/system/libs/helpers.bash: line 15: DOCKER_REG_CERTS_DIR: readonly variable test/system/libs/helpers.bash: line 16: DOCKER_REG_AUTH_DIR: readonly variable test/system/libs/helpers.bash: line 17: DOCKER_REG_URI: readonly variable test/system/libs/helpers.bash: line 18: DOCKER_REG_NAME: readonly variable test/system/libs/helpers.bash: line 21: PODMAN: readonly variable test/system/libs/helpers.bash: line 22: TOOLBX: readonly variable test/system/libs/helpers.bash: line 23: SKOPEO: readonly variable ... fedora-rawhide | 1..340 [1] bats-core/bats-core#904
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Apr 30, 2024
Ansible's built-in 'package' module doesn't show any details when installing the RPMs. All that can be seen is: TASK [Install RPM packages] fedora-rawhide | changed Therefore, there's no way to know what version of the packages got installed. In this case, not knowing the Bats version being used by the CI makes it difficult to know why the tests are generating this spew on Fedora Rawhide [1]: TASK [Run system tests] test/system/libs/helpers.bash: line 7: TEMP_BASE_DIR: readonly variable test/system/libs/helpers.bash: line 8: TEMP_STORAGE_DIR: readonly variable test/system/libs/helpers.bash: line 10: IMAGE_CACHE_DIR: readonly variable test/system/libs/helpers.bash: line 11: ROOTLESS_PODMAN_STORE_DIR: readonly variable test/system/libs/helpers.bash: line 12: ROOTLESS_PODMAN_RUNROOT_DIR: readonly variable test/system/libs/helpers.bash: line 13: PODMAN_STORE_CONFIG_FILE: readonly variable test/system/libs/helpers.bash: line 14: DOCKER_REG_ROOT: readonly variable test/system/libs/helpers.bash: line 15: DOCKER_REG_CERTS_DIR: readonly variable test/system/libs/helpers.bash: line 16: DOCKER_REG_AUTH_DIR: readonly variable test/system/libs/helpers.bash: line 17: DOCKER_REG_URI: readonly variable test/system/libs/helpers.bash: line 18: DOCKER_REG_NAME: readonly variable test/system/libs/helpers.bash: line 21: PODMAN: readonly variable test/system/libs/helpers.bash: line 22: TOOLBX: readonly variable test/system/libs/helpers.bash: line 23: SKOPEO: readonly variable ... fedora-rawhide | 1..340 [1] bats-core/bats-core#904 containers#1482
|
I underestimated the complexity. The code for counting tests and duplicates relied on having everything evaluated in the same shell. This will need a rework but I am out of time for today. |
Thanks for working on this, @martin-schulze-vireso ! |
martin-schulze-vireso
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
2 times, most recently
from
23f55be
to
85f73c2
Compare
Some editors like GNU Emacs insist on adding the newline at end of file, if it's missing, which leads to noise when looking at the actual code changes. bats-core#904
Test suites with a helper file with common constants that is loaded by multiple files stopped working with: $ bats /some/path /some/path/test_helper.bash: line 1: A_CONSTANT: readonly variable Error while sourcing library loader at '/some/path/test_helper.bash' If the helper file had anything else other than the common constants, then the tests would run, but the spew about the constants would still remain: $ bats --tap /some/path /some/path/test_helper.bash: line 1: A_CONSTANT: readonly variable 1..2 ok 1 first ok 2 second Fallout from 9d5ecdb bats-core#904
martin-schulze-vireso
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
from
240f074
to
c1cc818
Compare
Failures are due to scoping issues in traps that only happen when $() is used: bats-gather-tests:280, start of bats_gather_tests_source_exit_trap: local: can only be used in a function tracing.bash:301, end of bats_debug_trap: pop_scope: head of shell_variables not a temporary environment scope
martin-schulze-vireso
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
3 times, most recently
from
71820cc
to
d3b2e27
Compare
martin-schulze-vireso
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
from
d3b2e27
to
ec885f1
Compare
Windows and Linux go a newer version ( v18.20.3) with npm 10.7 (instead of 10.5)
martin-schulze-vireso
force-pushed
the
wip/rishi/unbreak-suite-load-readonly
branch
from
ec885f1
to
e406c01
Compare
martin-schulze-vireso
approved these changes
May 31, 2024
|
Thanks for your contribution. |
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.
Test suites with a helper file with common constants that is loaded by
multiple files stopped working with:
If the helper file had anything else other than the common constants,
then the tests would run, but the spew about the constants would still
remain:
Fallout from 9d5ecdb