SieveScript/test: Allow prepopulating variables so your sieve scripts can behave differently during test #5176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This way consumers can validate identifiers if they need to.
ksmurchison
approved these changes
Dec 18, 2024
wolfsage
commented
Dec 18, 2024
This way, a sieve script can behave differently in test mode based on provided parameters to the JMAP call.
Otherwise, a second SieveScript/test call in the same process will fail because template will be invalid. Also add logging if this happens so we can maybe figure out what went wrong.
This way SieveScript/test can reconstruct it properly
This will make it easy to test the SieveScript/test endpoint with various inputs and outputs.
... so you can specify a bunch of related tests together.
Also fill in all of the fileinto tests we need for now.
With this, you can say:
./testrunner Cyrus::JMAPSieve.sieve_test_extensive_fileinto*
to run all of the fileinto tests defined in this file
Otherwise we might try to install weird subroutines and it just won't end well.
The escaping of '\' was surprising and problematic when doing other types of tests, like inline jmapquery expressions. Rather than using that, escape manually when needed.
Without this, and jmapquerys that depend on the conversations db don't work. Also, we need to configure Cassandane::Cyrus::JMAPSieve to allow specific flags to be counted so we can use SomeInThreadHaveKeyword in our tests.
wolfsage
force-pushed
the
sieve-script-test-variables
branch
from
c22fc98 to
adcd55c
Compare
wolfsage
commented
Dec 20, 2024
| @@ -2257,7 +2259,6 @@ static int jmap_sieve_test(struct jmap_req *req) | |||
| } | |||
| } | |||
|
|
|||
| if (interp_ctx.cstate) conversations_commit(&interp_ctx.cstate); | |||
There was a problem hiding this comment.
The convdb is opened for us by jmap_api. If we commit it here, we'll crash later because jmap_api.c will try to commit it too
Avoid a crash when freeing the memory by freeing the strarray the same way all of the other strarrays are freed here. It appears that parts of the data are from a mmap'd read-only segment so they can't be freed with normal strarray_free().
ksmurchison
approved these changes
Dec 23, 2024
|
@rsto I've added include-in-fastmail to this one o/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.