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

Way for test classes, which use Expect #3925

Open
asmirnov-backend opened this issue Feb 16, 2025 · 3 comments · May be fixed by #3930
Open

Way for test classes, which use Expect #3925

asmirnov-backend opened this issue Feb 16, 2025 · 3 comments · May be fixed by #3930

Comments

@asmirnov-backend
Copy link
Contributor

While writing tests for issue #3853, I noticed that there are duplicates code in the test cases for the Expect functionality. For example, the following test cases share repetitive code fragments:

@Test
void throwsCorrectErrorForNewSizeAttrNaN() {
    MatcherAssert.assertThat(
        "the message in the error is correct",
        Assertions.assertThrows(
            ExAbstract.class,
            () -> new Dataized(
                new PhWith(
                    new EOmallocTest.PhiWithIdDummy(
                        new EOmalloc$EOof$EOallocated$EOresized()
                    ).it(),
                    "new-size",
                    new Data.ToPhi(true)
                )
            ).take(),
            "put TRUE in int attr fails with a proper message that explains what happened"
        ).getMessage(),
        Matchers.equalTo("the 'new-size' attribute must be a number")
    );
}

@Test
void throwsCorrectErrorForNewSizeAttrNotAnInt() {
    MatcherAssert.assertThat(
        "the message in the error is correct",
        Assertions.assertThrows(
            ExAbstract.class,
            () -> new Dataized(
                new PhWith(
                    new EOmallocTest.PhiWithIdDummy(
                        new EOmalloc$EOof$EOallocated$EOresized()
                    ).it(),
                    "new-size",
                    new Data.ToPhi(42.42)
                )
            ).take(),
            "put double in int attr fails with a proper message that explains what happened"
        ).getMessage(),
        Matchers.equalTo("the 'new-size' attribute (42.42) must be an integer")
    );
}

@Test
void throwsCorrectErrorForNewSizeAttrNotNatural() {
    MatcherAssert.assertThat(
        "the message in the error is correct",
        Assertions.assertThrows(
            ExAbstract.class,
            () -> new Dataized(
                new PhWith(
                    new EOmallocTest.PhiWithIdDummy(
                        new EOmalloc$EOof$EOallocated$EOresized()
                    ).it(),
                    "new-size",
                    new Data.ToPhi(-42)
                )
            ).take(),
            "put negative int in natural attr fails with a proper message that explains what happened"
        ).getMessage(),
        Matchers.equalTo("the 'new-size' attribute (-42) must be greater or equal to zero")
    );
}

These tests share a common structure and could be refactored to reduce duplication and improve maintainability.

I propose to depelop classes for testing Expect.Number, Expect.Int Expect.Natural in ExpectTest.java. For example:

/**
 * Test that dataization of Phi throws an error about an incorrect attribute which must use {@link Expect.Number}.
 *
 * @since 0.51
 */
public static final class Number {
    private final Phi phi;

    /**
     * Constructor.
     * @param phi The Phi object for dataization
     */
    public Number(Phi phi) {
        this.phi = phi;
    }

    /**
     * Perform the test.
     */
    public void it() {
        MatcherAssert.assertThat(
            "the message in the error is correct",
            Assertions.assertThrows(
                ExAbstract.class,
                () -> new Dataized(phi).take(),
                "fails with correct error message while transforming Phi to Number"
            ).getMessage(),
            Matchers.matchesRegex(".*the '.*' attribute must be a number.*")
        );
    }
}

So upper test will look:

@Test
void throwsCorrectErrorForNewSizeAttrNaN() {
    new ExpectTest.Number(
        new PhWith(
            new EOmallocTest.PhiWithIdDummy(
                new EOmalloc$EOof$EOallocated$EOresized()
            ).it(),
            "new-size",
            new Data.ToPhi(true)
        )
   ).it()
}

@yegor256 What do you think?

Ref: #3461

@yegor256
Copy link
Member

@asmirnov-backend maybe you can design custom matchers, as it's done here: https://github.com/objectionary/lints/tree/master/src/test/java/matchers

@maxonfjvipon
Copy link
Member

@yegor256 are you we want to go deeper with with Expect stuff? What about "safe" decorators for EO objects?

@yegor256
Copy link
Member

@maxonfjvipon I'm not yet sure. It seems that extra validation inside atoms will only make error messages more interesting, not anyhow contradicting with the public contracts of them. With the presence of Except we don't make any false promises to users. It's just to make testing more fun: atoms still fails if inputs are broken, but they fail with nicer messages.

Safe decorators is a different story -- they are visible to users and we definitely need them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants