|
Description
Marion Durand
2025-10-02 09:52:57 UTC
Created attachment 190995 [details] [review] Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl This patch adds unit tests for the new exclude_letter_code functionality in SendQueuedMessages, which will allow excluding specific letter codes when processing the message queue. Test coverage includes: - Excluding a single letter code (array format) - Excluding multiple letter codes - Excluding with scalar parameter format Created attachment 190996 [details] [review] Bug 40934: Add --exclude-code option to process_message_queue.pl Some libraries want to send digest messages once per day (e.g., at 20:05) but send all other messages every hour. Currently, this requires listing almost all letter codes (~85) in the hourly cron job, which is error-prone and makes it easy to forget notifications at upgrades. This patch adds a new --exclude-code (-x) option to process_message_queue.pl that allows excluding specific letter codes from processing, making it much easier to configure digest scheduling. Changes: - Added --exclude-code option to process_message_queue.pl (repeatable) - Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter - Supports both scalar and array reference formats Example usage: # Send all messages except digests every hour 00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \ --exclude-code PREDUEDGST --exclude-code HOLDDGST \ --exclude-code AUTO_RENEWALS_DGST # Send all messages including digests once per day 05 20 * * * process_message_queue.pl Test plan: 1. Apply both patches 2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t' 3. Verify all tests pass (should show 105 tests passing) 4. Create test messages in message_queue with different letter codes: - Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD 5. Test single exclusion: misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST - Verify DUEDGST messages remain pending - Verify other messages are processed 6. Test multiple exclusions: misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \ --exclude-code PREDUEDGST - Verify both digest messages remain pending - Verify non-digest messages are processed 7. Test combined with --code option (should work together): misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \ --exclude-code DUEDGST - Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway) 8. Test help output: misc/cronjobs/process_message_queue.pl --help - Verify --exclude-code option is documented Hello,
Thank you for your patch.
I tried to test but in the first step of the test plan, some test failed. Is it
On a brand new ktd on main, I applied both patches and used the command
`ktd --shell --run 'prove t/db_dependent/Letters.t'`
I have some test failed, I tried to restart_all then launch the test again but the results is the same. Is it a problem on my setup ?
Here is the full test results :
```
# Failed test 'PREDUEDGST message processed'
# at t/db_dependent/Letters.t line 1916.
# got: 'sent'
# expected: 'failed'
# Failed test 'ACQ_NOTIF message processed'
# at t/db_dependent/Letters.t line 1917.
# got: 'sent'
# expected: 'failed'
# Failed test 'TEST_MESSAGE message processed'
# at t/db_dependent/Letters.t line 1918.
# got: 'sent'
# expected: 'failed'
# Failed test 'ACQ_NOTIF message processed'
# at t/db_dependent/Letters.t line 1935.
# got: 'sent'
# expected: 'failed'
# Failed test 'TEST_MESSAGE message processed'
# at t/db_dependent/Letters.t line 1936.
# got: 'sent'
# expected: 'failed'
# Failed test 'ACQ_NOTIF message processed'
# at t/db_dependent/Letters.t line 1950.
# got: 'sent'
# expected: 'failed'
# Looks like you failed 6 tests of 10.
# Failed test 'Test exclude_letter_code parameter for SendQueuedMessages'
# at t/db_dependent/Letters.t line 1952.
# Looks like you planned 105 tests but ran 106.
# Looks like you failed 1 test of 106 run.
t/db_dependent/Letters.t ..
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/105 subtests
Test Summary Report
-------------------
t/db_dependent/Letters.t (Wstat: 256 (exited 1) Tests: 106 Failed: 2)
Failed tests: 105-106
Non-zero exit status: 1
Parse errors: Bad plan. You planned 105 tests but ran 106.
Files=1, Tests=106, 4 wallclock secs ( 0.02 usr 0.01 sys + 3.17 cusr 0.61 csys = 3.81 CPU)
Result: FAIL
```
(In reply to Marion Durand from comment #3) > Hello, > > Thank you for your patch. > I tried to test but in the first step of the test plan, some test failed. Is > it > > On a brand new ktd on main, I applied both patches and used the command > `ktd --shell --run 'prove t/db_dependent/Letters.t'` > I have some test failed, I tried to restart_all then launch the test again > but the results is the same. Is it a problem on my setup ? The tests fail for me as well, so it is not your setup. You can happily change the status to Failed QA, for Martin to fix up when he gets a chance 8-). Created attachment 191304 [details] [review] Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl This patch adds unit tests for the new exclude_letter_code functionality in SendQueuedMessages, which will allow excluding specific letter codes when processing the message queue. Test coverage includes: - Excluding a single letter code (array format) - Excluding multiple letter codes - Excluding with scalar parameter format Created attachment 191305 [details] [review] Bug 40934: Add --exclude-code option to process_message_queue.pl Some libraries want to send digest messages once per day (e.g., at 20:05) but send all other messages every hour. Currently, this requires listing almost all letter codes (~85) in the hourly cron job, which is error-prone and makes it easy to forget notifications at upgrades. This patch adds a new --exclude-code (-x) option to process_message_queue.pl that allows excluding specific letter codes from processing, making it much easier to configure digest scheduling. Changes: - Added --exclude-code option to process_message_queue.pl (repeatable) - Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter - Supports both scalar and array reference formats Example usage: # Send all messages except digests every hour 00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \ --exclude-code PREDUEDGST --exclude-code HOLDDGST \ --exclude-code AUTO_RENEWALS_DGST # Send all messages including digests once per day 05 20 * * * process_message_queue.pl Test plan: 1. Apply both patches 2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t' 3. Verify all tests pass (should show 105 tests passing) 4. Create test messages in message_queue with different letter codes: - Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD 5. Test single exclusion: misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST - Verify DUEDGST messages remain pending - Verify other messages are processed 6. Test multiple exclusions: misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \ --exclude-code PREDUEDGST - Verify both digest messages remain pending - Verify non-digest messages are processed 7. Test combined with --code option (should work together): misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \ --exclude-code DUEDGST - Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway) 8. Test help output: misc/cronjobs/process_message_queue.pl --help - Verify --exclude-code option is documented Created attachment 191306 [details] [review] Bug 40934: Add tests for exclude_letter_code in process_message_queue.pl This patch adds unit tests for the new exclude_letter_code functionality in SendQueuedMessages, which will allow excluding specific letter codes when processing the message queue. Test coverage includes: - Excluding a single letter code (array format) - Excluding multiple letter codes - Excluding with scalar parameter format Sponsored-by: OpenFifth <https://openfifth.co.uk/> Created attachment 191307 [details] [review] Bug 40934: Add --exclude-code option to process_message_queue.pl Some libraries want to send digest messages once per day (e.g., at 20:05) but send all other messages every hour. Currently, this requires listing almost all letter codes (~85) in the hourly cron job, which is error-prone and makes it easy to forget notifications at upgrades. This patch adds a new --exclude-code (-x) option to process_message_queue.pl that allows excluding specific letter codes from processing, making it much easier to configure digest scheduling. Changes: - Added --exclude-code option to process_message_queue.pl (repeatable) - Modified C4::Letters::SendQueuedMessages to handle exclude_letter_code parameter - Supports both scalar and array reference formats Example usage: # Send all messages except digests every hour 00 */1 * * * process_message_queue.pl --exclude-code DUEDGST \ --exclude-code PREDUEDGST --exclude-code HOLDDGST \ --exclude-code AUTO_RENEWALS_DGST # Send all messages including digests once per day 05 20 * * * process_message_queue.pl Test plan: 1. Apply both patches 2. Run: ktd --shell --run 'prove t/db_dependent/Letters.t' 3. Verify all tests pass (should show 105 tests passing) 4. Create test messages in message_queue with different letter codes: - Insert messages with codes: DUEDGST, PREDUEDGST, ACQ_NOTIF, HOLD 5. Test single exclusion: misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST - Verify DUEDGST messages remain pending - Verify other messages are processed 6. Test multiple exclusions: misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST \ --exclude-code PREDUEDGST - Verify both digest messages remain pending - Verify non-digest messages are processed 7. Test combined with --code option (should work together): misc/cronjobs/process_message_queue.pl --code ACQ_NOTIF \ --exclude-code DUEDGST - Verify only ACQ_NOTIF messages processed (DUEDGST excluded anyway) 8. Test help output: misc/cronjobs/process_message_queue.pl --help - Verify --exclude-code option is documented Sponsored-by: OpenFifth <https://openfifth.co.uk/> My mistake on the tests.. I commited too early.. They should all be passing now. Thank you for your work :) Everything works well for me except for the point 7. If I try to use both --code and --exclude-code, the order of the parameter matters. My message queue table contain 4 messages with status pending (restored between each run of process_mesage_queue). - PREDUE - DUEDGST - WELCOME - CHECKOUT a- If I use the command ``` ./misc/cronjobs/process_message_queue.pl --code WELCOME --exclude-code DUEDGST ``` The WELCOME notice is processed and nothing else is processed (expected for --code, the exclude-code is not needed as it will only process WELCOME notice) b- If I use the command ``` ./misc/cronjobs/process_message_queue.pl --exclude-code DUEDGST --code WELCOME ``` Everything is processed except DUEDGST (expected for --exclude-code, the --code is not taken into account (only the WELCOME should be processed if it was)) So the script is not warning, but only the first one is taken into account. To me, it makes no sens to use both argument --code and --exclude-code in the same line. (either I send everything except a few (--excludecode) or I send only those listed (--code)). In other word (because I'm not sure if I'm clear) The --code argument is used to send only a list of message (so no need to exclude some other, just list what you need) The --exclude-code argument is used to send everything except a list (so no need to list what you want to send). I think it should work the same as --category and --skip-category in cronjob longoverdue.pl (they are incompatible with each other). |