Allow per scenario differing ValueRetrievers in combination with the TableHelperExtensionMethods methods in parallel executed tests.

[ajeckmans]

Currently when using for instance Table.CreateInstance<SomeClass>() internally all ValueRetrievers are stored in a static collection. This means that the first call will fix the ValueRetrievers for the entire test run.

Given an application with a data store and a scenario such as the following:

Given the following customers exists
| Name |
| ABC  |
| DEF  |
When a request comes in to delete the customer ABC 
Then the following customers exist
| Name |
| ABC  |

When the record stored actually contains the data ABC then implementation is relatively straightforward. But imagine that the Customer record is actually just some long difficult string (for instance easdjkf87ewrkhjasdh789ewrkjsasf with some other non-relevant data. Writing those in scenario's is not something I would want.
So a translation from the identifier used in the scenario and the actual scenario is needed. We use a ValueRetriever (and a StepArgumentTransformation) that is given a per scenario container to generate a mapping between in this case ABC and easdjkf87ewrkhjasdh789ewrkjsasf, specifically:

[Binding]
public class MyHooks
{
    private MappingContext _context;

    public MyHooks(MappingContext context)
    {
        _context = context;
    }

    [BeforeScenario]
    public void SetupTestUsers()
    {
        Service.Instance.ValueRetrievers.Register(new CustomerNameRetriever(_context));
    }
}

The above is just a simplification of what we actually do, btw, but I hope the point comes across.

The above will work perfectly fine when all of the scenarios are executed in sequence as the Register call will overwrite the previous registration.
It breaks down when scenarios are executed in parallel as the ValueRetriever used most likely is given a context belonging to a different scenario, whereas the StepArgumentTransformation used will be given the proper context.

---

The issue is that the TEHelpers internally use the Reqnroll.Assist.Service.Instance method, which creates a single static instace that stores a list of ValueComparers and ValueRetrievers. A bit like how the ScenarioContext and FeatureContext used to be static as well.

I'd like to change this to something like a TableHelpers class that is injected through the DI. Which would mean that instead of writing table.CreateSet<>(), you'd write tablehelpers.CreateSet<>(table). The TableHelpers would then have an instance of the Service injected, which would make the ValueRetrievers changeable per scenario, I think.

Is this an approach that is viable?

(see also https://github.com/SpecFlowOSS/SpecFlow/issues/2492)

[gasparnagy]

This is a good initiative and I was also thinking about that already.

Without going into the details, basically the challenge is that tables are detached from the scenario execution context, but for performing the conversions we would need to have a scenario execution context. This is the reason why we cannot use [StepArgumentTransformation]s as well.

I see two approaches:

1. We make tables attached to the scenario execution context (in most of the cases the Table/DataTable instance is anyway created by Rqnroll, so it could immediately attach it to the context) and make retrievers/comparers scenario context aware (not a static method).
2. We move the CreateSet / CompareToSet / etc. methods to a service class (maybe to ScenarioContext), so they would be called like _scenarioContext.CreateSet<SomeClass>(myDataTable).

Both options have pros and cons... I don't know.

What do you think?

[gasparnagy]

Yes, definitely. The property injection is just an optional possibility.

[ajeckmans]

I failed to get the test suite working on my laptop, so I had to get a hold of another laptop., but I managed to get started working on a prototype.
Time during working hours will not be available until April, so it will be spare time only and thus a bit more slow going.

[gasparnagy]

No worries @ajeckmans. Thanks for your efforts. (Making the dev setup easier for contributors is one of my other priority goal BTW.)

[ajeckmans]

@gasparnagy see #108
I still cannot get all tests to run, but it is way better than it was two months ago!

[gasparnagy]

@ajeckmans I would like to learn about the ones that are not running, but let's wait for #109 to be merged first, because I hope that will further improve the situation.

[orifn1]

it looks like I have a similar issue with ValueRetrievers but another approach
#155

[ajeckmans]

yes, though this uses documented features, although ones that are not thread-safe and as such enforce that the tests cannot run in parallel anymore.

I've decided to explore an alternative approach to the one in this PR. That would enable us to fix having to create both StepArgumentTransformations and ValueRetrievers, which both have similar, but not entirely overlapping goals. If it will take too long I'm going to finish this PR though.

[orifn1]

Hello,
any updates?

[gasparnagy]

Not yet. But it is on the list. I think what we need is a "big redesign" of what we really want. I can help in that, but during the summer my availability is more limited, so maybe this will be more active again in September.