Commit graph

6 commits

Author SHA1 Message Date
Georg Lauterbach e6dee0f2f1
ci: move tests than can be run in parallel now (#3038) 2023-01-30 09:19:47 +01:00
Georg Lauterbach e3c4ef76c6
tests(refactor): Improve consistency and documentation for test helpers (#3012) 2023-01-22 00:05:28 +01:00
Brennan Kinney 8d80c6317f
tests(refactor): Adjust mail_changedetector + change detection helpers (#2997)
* tests(refactor): `mail_changedetector.bats` - Leverage DRY methods

`supervisorctl tail` is not the most reliably way to get logs for the latest change detection and has been known to be fragile in the past.

We can instead read the full log for the service directly with `tac` and `sed` to extract all log content since the last change detection.

Common asserts have also been extracted out into separate methods.

* tests(chore): Remove sleep and redundant change event

Container 1 is still blocked at this point from an existing lock and change event.

Make the lock stale immediately and no extra sleep is required when paired with the helper method to wait until the event is processed (which should remove the stale lock).

* tests(refactor): Add more DRY methods

- Simplify the test case so it's easier to grok.
- 2nd test case (blocking) extracts out initial setup into a separate method and merges the later service restart logic which is redundant.
- Additional comments for improved context of what is going on / expected.

* tests(chore): Revise the change detection helper method

- Add explicit counting arg to change detection support.
- Extract revised logic into it's own generic helper method.
- Utilize this for a separate method that monitors for a change event having started, but not waiting for completion.

This allows dropping the 40 sec of remaining `sleep` in `mail_changedetector` test. It was also required due to potentially missing the timing of a change event completing concurrently in a 2nd container that needed to be waited on and then checked.

* tests(chore): Migrate to current test conventions

- Switch to common container setup helpers
- Update container name and change usage to variables instead.
- Adopt the new convention of prefix variable for test cases (revised test case descriptions).

* tests(chore): Remove legacy change detection

This has since been replaced with the new helper watches the `changedetector` service logs directly instead of only detecting a change has occurred via checksum comparison.

No tests use this method anymore, it was originally for `tests.bats`. Thus the tests in `test_helper.bats` are being dropped too. The new helper has test coverage in `changedetector` tests.

* chore: Lock removal should not incur `sleep 5` afterwards

- A new lock should be created by this script after removal. The sleep doesn't help avoid a race condition with lock file creation after removal.
- Reduces test time as a bonus.
- Added some additional comments to test.

* tests(chore): `tls_letsencrypt.bats` leverage improved change detection

- No need to wait on the change detection service anymore during container startup.
- No need to count change events processed either as waiting a fixed duration is no longer relied on.
- This makes the reload count method redundant, dropped.

* tests(chore): Convert `setup-cli.bats` to new test conventions

This test file was already adapted to the original common setup helpers.

- `TEST_NAME` replaced with `CONTAINER_NAME`.
- Prefix var added, test case descriptions drop explicit prefix.
- No other changes.

* tests(chore): Extract out helpers related to change-detection

- New helper file for sharing these helpers to tests.
- Includes the helpful log method from changedetector tests.
- No longer need to maintain duplicate copies of these methods during the test migration. All tests that use them are now importing the separate helper file.
- `tls_letsencrypt.bats` has switched to using the log helper.
- Generic log count helper is removed from `test_helper/common.bash` as any test that needs it in future can adapt to `helper/common.bash`.

* tests(refactor): `tls_letsencrypt.bats` remove `_get_service_logs()`

This helper does not seem useful as moving away from `supervisorctl tail` and no other tests had a need for it.

* tests(chore): Remove common setup methods from `test_helper/common.bash`

No other tests depend on this. Future tests will adopt the revised versions from `helper/setup.bash`.

Additionally updates `helper/setup.bash` comments that are no longer applicable to `TEST_TMP_CONFIG` and `CONTAINER_NAME`.

* chore: Use `|| true` to simplify setting `EXPECTED_COUNT` correctly
2023-01-16 20:39:46 +13:00
Brennan Kinney b58165762a
fix(changedetector): Use service reload commands instead of supervisorctl restart <service> (#2947)
With `reload` a change detection event during local testing can be processed in less than a second according to logs. Previously this was 5+ seconds (_plus additional downtime for Postfix/Dovecot to become available again_).

In the past it was apparently an issue to use `<service> reload` due to a concern with the PID for wrapper scripts that `supervisorctl` managed, thus `supervisorctl <service> restart` had been used. Past discussions with maintainers suggest this is not likely an issue anymore, and `reload` should be fine to switch to now 👍 

---

**NOTE:** It may not be an issue in the CI, but on _**local systems running tests may risk failure in `setup-cli.bats` from a false positive**_ due to 1 second polling window of the test helper method, and a change event being possible to occur entirely between the two checks undetected by the current approach.

If this is a problem, we may need to think of a better way to catch the change. The `letsencrypt` test counts how many change events are expected to have been processed, and this could technically be leveraged by the test helper too.

---

**NOTE:** These two lines (_with regex pattern for postfix_) are output in the terminal when using the services respective `reload` commands:

```
postfix/master.*: reload -- version .*, configuration /etc/postfix
dovecot: master: Warning: SIGHUP received - reloading configuration
```

I wasn't sure how to match them as they did not appear in the `changedetector` log (_**EDIT:** they appear in the main log output, eg `docker logs <container name>`_).

Instead I've just monitored the `changedetector` log messages, which should be ok for logic that previously needed to ensure Dovecot / Postfix was back up after the `restart` was issued.

---

Commit history:

* chore: Change events `reload` Dovecot and Postfix instead of `restart`

Reloading is faster than restarting the processes.

Restarting is a bit heavy handed here and may no longer be necessary for general usage?

* tests: Adapt tests to support service `reload` instead of `restart`

* chore: Additional logging for debugging change event logs

* fix: Wait on change detection, then verify directory created

Change detection is too fast now (0-1 seconds vs 5+).

Directory being waited on here was created near the end of a change event, reducing that time to detect a change by the utility method further.

We can instead check that the directory exists after the change detection event is completed.

* chore: Keep using the maildir polling check

We don't presently use remote storage in tests, but it might be relevant in future when testing NFS.

This at least avoids any confusing failure happening when that scenario is tested.
2022-12-24 01:57:24 +13:00
Brennan Kinney 835056d707 tests(chore): Use REPOSITORY_ROOT export var from Makefile
Allows for using `load` with an absolute path instead of a relative one, which makes it possible to group tests into different directories.

Parallel tests differ slightly, loading the newer `helper/common.bash` and `helper/setup.bash` files instead of the older `test_helper/common.bash` which serial tests continue to use.
2022-11-26 14:52:42 +13:00
Georg Lauterbach 59127e2b25 tests(chore): Rename test files to serial and parallel types
- `test_helper.bats` needs more work than this PR provides to be compatible with parallel tests, so must remain as a serial test for now.
- `spam_bounced.bats` had failures as a serial test, but works well converted to a parallel test in a future commit.
2022-11-26 14:52:42 +13:00
Renamed from test/mail_changedetector.bats (Browse further)