-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
ActionChain shrink should be called only with state objects that correspond to the relevant state position #428
Comments
I'm afraid it boils down to In other words, it is similar to #427, however, the difference is that the offending method is |
The example above is
|
It looks like the ways out are: b) Variation of "a": first execute the chain as usual (no snapshotting, no shrinking), however, if the chain fails, then re-execute it and capture several c) Delay shrinking till "re-execution" of the chain. In other words, when re-executing, try shrinking some of the actions before executing them. The difference from "a" is that we can't really weigh the shrink result before we start re-execution. d) Enforce the state to be serializable (or externalizable in some way). Then the engine could snapshot the state before executing each action, and then it could restore from those snapshots in cases shrink is needed. e) Enforce users to clone all the data they need when producing state-dependent transformations. f) Variation of "e": enforce immutable approach for the state. For instance, if the user uses only immutable collections for the state (e.g. https://github.com/usethesource/capsule, https://github.com/andrewoma/dexx), then they are guaranteed to have an immutable state. WDYT? For now, I incline to variation One of my actions takes ~2 minutes, so I think it should be just fine to keep several shrink results in memory for every action even in case the chain succeeds. |
…ccess state model state during shrink phase This might take slightly more memory and produce suboptimal shrinks, however makes shrink possible for state-accessing transformations. Fixes jqwik-team#428
…ccess state model state during shrink phase This might take slightly more memory and produce suboptimal shrinks, however makes shrink possible for state-accessing transformations. Fixes jqwik-team#428
…access state model state during shrink phase This might take slightly more memory and produce suboptimal shrinks, however makes shrink possible for state-accessing transformations. Fixes jqwik-team#428
@vlsi Could you add the code for SetMutatingChainState if you have it still lying around? Would simplify playing around with the options a bit. |
Added |
I think a) is not a feasible option since prefetching shrinking results can access the state in behaviour changing way. This could lead to additional queries to a database or other external resources and thereby completely change the stateful object's behaviour. The same holds for b) unless the chain would be executed for each shrinking attempt. c) would - as you note - require a different overall shrinking approach So my current idea is to move in two steps:
Knowing to work with immutables would also make allow more powerful shrinking in some cases, |
All my actions access state, so discarding the tail after shrinking one of the elements is virtually useless as it would be almost equivalent to executing the head of the chain.
Technically speaking, my state is mutable: a part of the state is within mutable objects, and a part of the state is in the database; so “is immutable” flag won’t help me.
I think it is reasonable to ask that .shrink methods should not mutate the state. Just like producing actions should not mutate the state even if it accesses the state for producing action. So prefetching shrink results is a workable from the technical point of view.
d is exactly about requiring the user to implement serialiser and deserialiser, so it is possible in Java modulo the fact that user would have to code it. |
However, it looks like shrinking of the individual actions is not helpful for stateful chains. For instance, my case is somewhat comparable to testing a file system: one can create and remove files and folders. In other words, even if someone implements serializer-based or immutable-based shrink improvements, it won’t really help me as the shrinking time would be unacceptably long. I wonder if there could be a design where actions refer to another actions. Suppose that Arbitrary that produces createFile(parentFolder: Folder, name: String) would stick to an action that generated the parent folder rather than selecting a folder from a “all folders” collection. |
Actions could still be shrunk away, just not be shrunk individually. |
State in Java can potentially have so many side-effects that relying on any library to fully handle it, it just not feasible. In the example above, I'd probably switch from naive state-based properties to a model-based approach (https://johanneslink.net/model-based-testing). If the model itself is implemented as immutable object than most problems just disappear.
Which gets us to #300. Haven't checked if it can be done as easily with the new stateful properties. In general, putting more implementation complexity on top of the existing one - like pre-calculating a configurable amount of shrinks or dealing with hand-made (de-)serializers - to just cover a few more edge cases but not solving the overall problem, is not where I want to go at the moment. That probably means that I will release 1.7.2 without a solution to this issue. |
What do you mean? |
The access to the wrong state happens during individual shrinking - shortest filename etc. That's the part that leads to the |
I don't understand how immutability or "model-based-testing" improves shrinking for cases like "testing filesystem with files and folders". |
That's because the example uses the old stateful API. With the new one it could directly choose from an existing user. The article also does not implement an immutable model, which would be necessary for working well with the new API. The advantage of an explicit model is that all shrinking can be done without accessing the real thing (eg a database or a file system). Thus, it's usually faster and does not have side effects. |
That is helpful, however, I would consider shrinking individual names only after the eninge figures out the minimal number of steps to reproduce the failure. It just makes no sense trying to shrink all individual file names, especially, if the file was not needed for reproducing a bug. Here's a bug I have in mind: suppose a filesystem crashes when two different folders contain file with the same name. The chain that reproduces the crash might look like
I would like that shrinker removes |
How? I really do not understand it. I use the new stateful API, and I use |
I think you make my case :-) That's why I consider the safe guard I suggested as first implementation change still worthwhile. It will only prevent individual shrinking of actions not shrinking them away. |
But you do not use an explicit, immutable model, right? Your actions are based on the real SUT's state as far as I understand. Which leads to many (or all) of the potential shrinking problems. |
My current state is a bunch of Frankly speaking, I believe another approach would be "manual" shrinking. |
That's maybe a new and interesting approach. I haven't seen a structural approach to stateful generations of actions anywhere. Go for it! If it's best implemented on top of chains or on top of plain jqwik, I cannot really tell. Most complexity in chain implementation comes from shrinking, so I assume that manual shrinking could do without some of it. |
I mean more like unparse to a structured form rather than "use structural approach for generation". For reference, here's a test case crafted manually. open class SeveralTypeColumnTestSuite : BaseTest() {
@Test
override fun test() {
val mapping = createMapping<Mapping>()
mapping.applyModifyAndApply {
table.configure { // This is effectively "add column to table" action
"TEXT"(textAttribute)
}
}
}
open class InitialMapping : BaseMapping() {
lateinit var table: PlainTable
val textAttribute by textAttribute()
val type1 by objectType()
val type2 by objectType()
override fun Mapping.init() {
configFiles {
configFile("test") {
statistics {
statistic(attribute = textAttribute, numRows = 100)
statistic(objectType = type1, numRows = 100)
statistic(objectType = type2, numRows = 100)
}
odb {
tables {
table("TEST_UT_S__MAP_TO_OT1") {
+type1
"TYPE"(ObjectField.OBJECT_TYPE_ID)
}
table = table("TEST_UT_S__MAP_TO_OT2") {
+type2
"TYPE"(ObjectField.OBJECT_TYPE_ID)
"TYPE2"(ObjectField.OBJECT_TYPE_ID)
statistics(numRows = 100)
threshold(attribute = 1, objectType = 1, table = 1)
}
}
}
}
}
}
}
} |
I've created #447 for tracking "generate test source code" feature. |
Here's a sketch of my idea to allow more differential specification of transformer based stateful chains. Introduction of new class class ChainSpec<T> {
boolean isImmutable;
boolean hasSideEffects;
Supplier<ChangeDetector<T>> detectorSupplier;
// Maybe more attributes eg Serializer/Deserializer for state
// Some static factories, e.g.
static <T> ChainSpec<T> immutable() { ... }
static <T> ChainSpec<T> mutableWithoutSideEffects() { ... }
} Default would be mutable, with side effects and no change detector. This covers everything, but has the worst shrinking behaviour. Now, class Chain {
static <T> ChainArbitrary<T> startWith(Supplier<? extends T> initialSupplier, ChainSpec<T> spec) {...}
} With spec in place we could get rid of |
I am indifferent to ChainSpec, as it does not help shrinking in my case. |
I think it could in two ways:
|
I do not see how
I do not see how it helps. I described a sample case that is close to the testing problem I have: #428 (comment) |
I tried out your file system example with an immutable model here: https://github.com/jlink/pbt-stateful-examples |
Of course, wrong-state object issue goes away, however, shrinking goes away as well 🤷 |
This issue is about fixing the bug without making shrinking worse. That’s what the approach achieves. |
That's fair. For now, I am more interested in exploring the way shrinking could survive removal of elements in collections. |
Closing since jqwik 2 will generate actions more robustly and with (hopefully) much better shrinking behaviour. |
Testing Problem
It looks like
.shrink()
can be called on a wrong state instance.The key idea behind the test:
add
int to a set, andclear
the setset
. Note: it selects the value to print based on thestate.set
, so I assume it should select valid values only.If the value is not present in the "print transformer", then I believe it is illegal.
The following test throws
IllegalStateException
The text was updated successfully, but these errors were encountered: