Deprecate ExpectedException attribute. about testfx HOT 12 OPEN

@robdalsanto you can specify a specific exception type

Assert.ThrowsException<T>(Action action)
Assert.ThrowsExceptionAsync<T>(Func<Task> action)

from testfx.

@jimontheriver I agree. ExpectedException would probably be tagged with an obsolete tag in a future release. So building a project with an ExpectedException would start throwing warnings.
/cc @pvlakshm.

from testfx.

I was actually thinking it would be better to extend it so that you can be more specific with the exception. i.e. permit specifying the error message (and/or the paramName). The attribute can save a lot of boilerplate code in tests when you are testing multiple failure paths.

I agree though that, as is, its not as accurate as it could be. For instance, consider this code:

public void DoWork(Person person)
{
    if (person == null) throw new ArgumentNullException(nameof(person));
    if (string.IsNullOrWhiteSpace(person.Forename)) throw new ArgumentException($"{nameof(person.Forename)} should be specified", nameof(person));
    if (string.IsNullOrWhiteSpace(person.Surname)) throw new ArgumentException($"{nameof(person.Surname)} should be specified", nameof(person));
    if (string.IsNullOrWhiteSpace(person.Email)) throw new ArgumentException($"{nameof(person.Email)} should be specified", nameof(person));
}

[TestMethod, ExpectedException(typeof(ArgumentException))]
public void WhenSurnameIsNull_ExceptionThrown()
{
    var person = new Person() { Forename = "Test" };
    DoWork(person);
}

As is, there is no guarantee that the exception is being thrown on the property I'm intending. If someone inadvertently swaps the checks around, or adds a new property, this test could still pass even though its not on the correct check.

To do this without the attribute requires something like this:

[TestMethod, ExpectedException(typeof(ArgumentException))]
public void WhenSurnameIsNull_ExceptionThrown()
{
    var person = new Person() { Forename = "Test" };
        try
        {
            DoWork(person);
        }
        catch (ArgumentException ex)
        {
            Assert.IsTrue(ex.ParamName == nameof(Person.Surname)
                && ex.Message == $"{nameof(Person.Surname)} should be specified");
        }
}

But by extending the attribute, you could do this:

[TestMethod] 
[ExpectedException(typeof(ArgumentException), ParamName := "person", $"{nameof(Person.Surname)} should be specified")]
public void WhenSurnameIsNull_ExceptionThrown()
{
    var person = new Person() { Forename = "Test" };
    DoWork(person);
}

This lets developers be much more specific with their Exception Testing. Overloading the constructors will allow this enhanced behaviour without breaking existing tests.

from testfx.

We make use of this attribute, so please keep it. That you can specify a specific exception type is meeting our requirements for avoiding false 'test pass' situations.
Without this, we would wind up writing our own try-catch-verify helpers.
--- edit Sept 18, 2108 ---
Thanks @ziomyslaw, I failed to notice the Assert.ThrowsException. Will plan to move to that.

from testfx.

I suggest we make the attribute obsolete and provide a code refactoring to automatically rewrite the code to use new/better APIs.

from testfx.

@fforjan Sadly there are none but I really want to add them! I was expecting to be working on that early this year but priorities have changed

from testfx.

After a couple of internal discussions, we want to try to be more strictly following semantic versioning as we have some users with huge test suite and this kind of change would lead to many failures and some time to fix or ignore. Pushing back to v4 as it will allow breaking changes.

from testfx.

We would need to fix the intellitest wizard that outputs ExpectedException in-tandem with this change so as not to break that workflow.

from testfx.

@Evangelink do you already have the code analyser in this repository for the refactoring tool ? I was looking for it but could not found it ?

from testfx.

We will deprecate the attribute as part of 3.3.0, the analyzer is added as part of 3.2.0.

from testfx.

@Evangelink you self-assigned this ticket in December. It still has the Help-Wanted label. I assume that label is no longer applicable and should be removed?

from testfx.

Hi @Kissaki,

I wanted to release v4 a long time ago but we decided to postpone its release so we can group more breaking changes to avoid doing many breaks increments.

We have already provided the analyzer so we only need to drop the attribute in v4.

from testfx.