Skip to content
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

Allow using retest ID locator without GM #570

Open
beatngu13 opened this issue Mar 20, 2020 · 1 comment
Open

Allow using retest ID locator without GM #570

beatngu13 opened this issue Mar 20, 2020 · 1 comment
Labels
enhancement New feature or request

Comments

@beatngu13
Copy link
Contributor

Problem

Using the retest ID as a locator is currently only possible if a GM exists. It would be helpful if it could also be used without a GM, which requires RetestIdProvider to be deterministic.

Solution

@roesslerj proposed a draft via #563, but this still needs some work. Quote from my comment:

Having a deterministic RetestIdProvider is not a contract to my knowledge, so we cannot rely on this. For example, we still ship RandomSuffixRetestIdProvider and the user is free to provide a custom implementation. Then this doesn't work.

If we want RetestIdProviders to be deterministic in general, we have to document and require this contract (like e.g. equals requires an equivalence relation), otherwise there will be undefined behavior.

Also, what happens if slight state changes lead to different counts? We no longer lookup the element in the old state and do a 1-on-1 assignment, instead we select it solely on the basis of the retest ID I guess?

It would rather mark this as a draft. We have to discuss the determinism and we need proper tests, at least for the aforementioned situations. If we do such a "drastic" change without tests, and no existing tests are failing, we are doing something wrong.

@beatngu13 beatngu13 added the enhancement New feature or request label Mar 20, 2020
@diba1013
Copy link

This has been partly addressed with #612, where it for now tries to find a retest ID, if no Golden Master has been created yet. This implementation is similar (if not equal) to the originally proposed solution and does no assumptions or semi-intelligent lookups. It simply queries the soon-to-be-persisted state for the retest ID instead of loading the previous one. Therefore it is the users responsibility to ensure that the retest ID is present.

Since then, I have documented the generation in more detail, but we should document this specific feature with its effects more thoroughly before release (e.g. that the default implementation is in fact deterministic).

Of course, the use cases are fairly limited, when not precisely knowing how the active RetestIdProvider generates the IDs. In such a case it would only come down to regenerating Golden Masters (assuming none of the queried attributes has changed, e.g. tag).

I will keep this open, until the feature has been documented. However, feel free to create more issues regarding your other points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants