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

update BATS & helper + minor updates to BATS variables #2988

Merged
merged 5 commits into from
Jan 9, 2023

Conversation

georglauterbach
Copy link
Member

@georglauterbach georglauterbach commented Jan 8, 2023

Description

NOTE: Please review commit by commit :)

BATS core:
--> previous: 1.7.0
--> now: 1.8.2

BATS assert:
--> previous: ffe84ea5dd43b568851549b3e241db150c12929c
--> now: 2.1.0

Type of change

  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

BATS core:
  previous: 1.7.0
  now:      1.8.2

BATS assert:
  previous: ffe84ea5dd43b568851549b3e241db150c12929c
  now:      2.1.0
@georglauterbach georglauterbach added area/tests kind/update Update an existing feature, configuration file or the documentation labels Jan 8, 2023
@georglauterbach georglauterbach added this to the v12.0.0 milestone Jan 8, 2023
@georglauterbach georglauterbach self-assigned this Jan 8, 2023
Move from `TEST_NAME_PREFIX` to `BATS_TEST_NAME_PREFIX` which BATS will
automatically prepend to tests in a given file. Didn't know why I did
not see this earlier.

The prefix names were made uniform too.
@georglauterbach georglauterbach changed the title update BATS & helper update BATS & helper + minor updates to BATS variables Jan 8, 2023
The test would very sporadically fail in case Amavis could not print its
log in-time. We will now wait at most 5 secs for Amavis to print its
log.
search-and-replace did not catch everything the last time
Copy link
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

Oh I somehow missed the assert lib finally getting a tagged release (I had been subscribed to the issue closed in Oct 2022) we could use instead of a master branch commit, thanks for sorting this out 🎉

Comment on lines +55 to +56
@ shopt -s globstar ; ./test/bats/bin/bats $(BATS_FLAGS) \
--no-parallelize-within-files --jobs $(BATS_PARALLEL_JOBS) test/$@/**/*.bats
Copy link
Member

@polarathene polarathene Jan 8, 2023

Choose a reason for hiding this comment

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

Prefer vertically listing the explicit options, easier to grok and track changes 😅

Suggested change
@ shopt -s globstar ; ./test/bats/bin/bats $(BATS_FLAGS) \
--no-parallelize-within-files --jobs $(BATS_PARALLEL_JOBS) test/$@/**/*.bats
@ shopt -s globstar ; ./test/bats/bin/bats $(BATS_FLAGS) \
--no-parallelize-within-files \
--jobs $(BATS_PARALLEL_JOBS) \
test/$@/**/*.bats

Side-notes (nothing below requiring change for this PR)

A reminder that --no-parallelize-within-files prevents --jobs 1 for running a parallel set sequentially.

Although I wouldn't worry about this much as I'm doubtful the default of --jobs 2 will cause anyone running these tests a problem, since we can still run a few individual tests with make test/file1,file2,file3 easily enough.


If of interest to you:

Copy link
Member Author

@georglauterbach georglauterbach Jan 9, 2023

Choose a reason for hiding this comment

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

I have merged this PR to make CI pass again, but we can tackle the suggestions in another PR - I don't really mind the changes to the Makefile since it's only a stylistic change :)

Comment on lines +21 to 26
@test "SpamAssassin integration should be active" {
# give Amavis just a bit of time to print out its full debug log
run repeat_in_container_until_success_or_timeout 5 "${CONTAINER_NAME}" grep 'ANTI-SPAM-SA' /var/log/mail/mail.log
assert_success
assert_output --partial 'loaded'
refute_output --partial 'NOT loaded'
Copy link
Member

Choose a reason for hiding this comment

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

This is works too 👍 (I noticed this locally and was going to use an existing helper method to wait for the amavis service to be ready)

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, instead of "grep-ing until success", grep until a string appears:

tail -F /var/log/mail/mail.log | grep --max-count 1 'ANTI-SPAM-SA'

timeout 5 tail -F /var/log/mail/mail.log | grep --max-count 1 'ANTI-SPAM-SA'

Copy link
Member

@polarathene polarathene Jan 10, 2023

Choose a reason for hiding this comment

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

grep until a string appears

I have been refactoring / debugging a test of the past couple days where a similar requirement with counting was necessary and being a hassle for me to shell-fu my way through, I may want to switch to your tip here now 😂

Copy link
Member

Choose a reason for hiding this comment

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

fyi: if you care for the output, use --max-count 1, otherwise just -q (grep will then quit after first match)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tests kind/update Update an existing feature, configuration file or the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants