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

Recursively call setup_suites in subdirectories #979

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

gwangmu
Copy link

@gwangmu gwangmu commented Aug 22, 2024

Thank you for maintaining BATS. :)

I modified bats-exec-suite so as to execute {setup,teardown}_suite on every subdirectory on a recursive run. The basic idea is to replace the file execution loop (calling bats-exec-file) with a tail recursion and to enter a subshell whenever the next test is in the subdirectory. In the subshell, it calls setup_suite of that subdirectory (if it exists), executes the test, and calls teardown_suite before changing the directory.

The motivation was that, if somebody (like, me) tries to organize their tests using the directory structure, the tests in the subdirectory probably need to assume the test environment of their parent, PLUS their own environment if needed. This is doable if BATS recursively recognizes {setup,teardown}_suites in the subdirectories.

That said, there are some concerns even from my side. I think these need to be addressed before this PR is eventually merged into the master.

  • Overhead

This is the most obvious concern. I totally agree with BATS's basic policy of being lightweight. I ensured this modification causes no additional overhead when it's not recursive, but I don't know whether a tail recursive is sizably heavier than a loop. For reference, the time difference between the unmodified and modified ones was negligible using the test directory structure in test/fixture/suite/recursive_setup.

  • The term 'suite' in setup_suite

I think this can be a little misnomer when this recursive setup takes effect. A 'suite' arguably means the set of tests, namely all the tests that BATS will execute. From this perspective, 'setup_suite' should be called only once before the entire test begins. However, I need to clarify that the motivation signified something "between" setup_file and setup_suite at a directory level. Maybe there can be several solutions: sticking to setup_suite for backward compatibility, newly creating the concept of setup_dir, or aggressively changing the name setup_suite to setup_dir.

  • Parallel execution

My implementation doesn't support parallel execution. There's nothing impossible to have this "feature" in parallel execution, but it'll definitely need some engineering effort.

I also need to make it clear that I ran all tests in test (i.e., test.bats and suite.bats) and confirmed that they all passed. I also added a new test for this PR in the test directory.

Thank you. :)

@gwangmu gwangmu requested a review from a team as a code owner August 22, 2024 15:02
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