Commit graph

3 commits

Author SHA1 Message Date
Brennan Kinney b7bad82e07
tests(fix): wait_until_change_detection_event_completes to count (#2974) 2023-01-04 13:29:10 +01: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 32cc9d30e5 tests(refactor): common.bash helper split into two files
The current `test/test_helper/common.bash` was getting large. Setup logic has been extracted out into a new file.

`common.bash` resides in a directory  named `test_helper/`, the `test_` prefix is redundant. 

As an interim solution this provides a new approach for the updated tests, while the "old" tests can use the "old" `common.bash`. Eventually all tests should migrate to the new approach in `helper/` instead of the older `test_helper/`.

The new helper files are located under `test/helper/` (_which drops the `test_` prefix_). The new and updated helpers apply the new naming convention for ENV variables (_such as `CONTAINER_NAME` or `IMAGE_NAME`_).

---

Some refactoring occurred, including new methods like `_run_in_container()` and `_default_teardown()`.

---

I encountered a situation before in which the updated tests would fail because there were collisions of ENV names in the tests (_for example with `CONTAINER_NAME`_).
2022-11-26 14:52:42 +13:00