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

tests: Update submodules for bats #2715

Merged

Conversation

polarathene
Copy link
Member

Description

This PR updates bats submodules. Notably the bats-assert and bats-support submodules have since been migrated to the bats-core organization, while our current remote has been unmaintained since 2016.

  • bats-core our last release of 1.4.1 (July 2021) updated to 1.7.0 (May 2022). Some fixes and new setup_suite() + teardown_suite() options to run once before and after all individual test files (setup_file() / teardown_file()) are handled.
  • bats-assert has had a fair amount of updates. I've opted to update to the current master commit as no official release has been tagged since Nov 2018. There is a request for a new tagged release since Jan 2021. There is some new assertions since the last release, such as assert_not_equal.
  • bats-support has had no new relevant activity for us since.

Type of change

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

Checklist:

  • New and existing unit tests pass locally with my changes

These two submodules were migrated to the `bats-core` organization, where they continued to receive updates.
This is technically one commit backwards, but no relevant difference has been made since, other than moving the submodule to the `bats-core` organization.
No official release tag since Nov 2018, but a fair amount of changes since then.
@polarathene polarathene added area/ci priority/low area/tests kind/update Update an existing feature, configuration file or the documentation labels Aug 10, 2022
@polarathene polarathene added this to the v11.2.0 milestone Aug 10, 2022
@polarathene polarathene self-assigned this Aug 10, 2022
@georglauterbach

This comment was marked as resolved.

@martin-schulze-vireso

This comment was marked as resolved.

@polarathene

This comment was marked as resolved.

Assertions often depend on the output of the last `run` command.

These tests were either missing a `run`, or incorrectly asserting against the wrong command they were intended for.

`wait_for_service` should not be asserted. It is just a pre-requisite before the actual test case is ready to proceed.

Updating bats and submodules helped surface this testing bug.
These commands used `run` with no assertion. If they were meant to assert success, they should include `assert_success` after each `run`.

Until a failure occurs, it is assumed these commands are not under test, and just part of setting up the test case.
@polarathene
Copy link
Member Author

That should fix the failures. Additional information regarding those changes is available in the commit messages.

@georglauterbach
Copy link
Member

I am currently re-running the tests as there were some weird quota errors. We'll see what happens in the second run.

This test was attempting to add the same accounts as in the previous test, which failed.

It was undetected previously due to the error being caught by `run` with no assertion to detect it.
@polarathene
Copy link
Member Author

polarathene commented Aug 11, 2022

Tests are passing now.

  • The update from this PR revealed the mishandled assert calls.
  • I removed the unnecessary run usage, which was consuming the errors silently.
  • Fixed the tests 👍

@polarathene polarathene merged commit 0d5f550 into docker-mailserver:master Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci area/tests kind/update Update an existing feature, configuration file or the documentation priority/low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants