IntelliTect / TestTools

A collection of tools for aiding in test automation
MIT License
10 stars 7 forks source link

Adding Dispose to browser #51

Closed PandaMagnus closed 5 years ago

PandaMagnus commented 5 years ago

Resolves #50

PandaMagnus commented 5 years ago

@Keboo I didn't consider what would happen if the class is derived from (which, while I personally haven't hit a use case, I can definitely imagine some scenarios where that could occur). I'll go ahead and update to follow the BaseClass example.

PandaMagnus commented 5 years ago

@Keboo I didn't consider what would happen if the class is derived from (which, while I personally haven't hit a use case, I can definitely imagine some scenarios where that could occur). I'll go ahead and update to follow the BaseClass example.

@Keboo It's not clear to me from the documentation, would implementing my own SafeHandle be a good option here? That seems weird to me, since really I just need to clean up Selenium in a way that common patterns understand (e.g. if they invoke Browser.cs in a using statement, or if they're using a service provider, or etc.)

Keboo commented 5 years ago

Was picturing something much simpler like this:

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        Driver?.Dispose();
    }
}

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}
PandaMagnus commented 5 years ago

@Keboo Oh dude, that's way simpler. Okay cool, I can deal with that. :) Do you still think it's worth saving off whether it's been disposed or not and just returning if it has?

Keboo commented 5 years ago

Yea, you can if you want. The key details is that calling Dispose multiple times should be handled. Exactly how that gets done is an implementation detail.

PandaMagnus commented 5 years ago

@Keboo Done and done (I think. :) )