review: test: improve tests for abstractTemplate

[Rohitesh-Kumar-Jain]

#1818

Hi, I have improved both the test strength and mutation coverage from 75% to 100% class.

Here's the report before improvement.

This PR was actually supposed to be raised in week 1, but I had to wait for tests to get converted in Junit5.

What I believe is, unhappy path test was missing for isWellFormed function which led to poor test strength in Descartes report, so I wrote a test for an unhappy path to fix that.

Next, for this week I will be working on the Substitution.java which is 660 lines of codes with many methods, so I believe I will raise another PR for that after a couple of days.

Link to #3967

[slarse]

Very good! I've confirmed that the new test does kill at least one mutant, so that's excellent.

Aside from the comments on the code, I also have one general comment: you don't necessarily need to mark the arrange/act/assert phases explicitly. In smaller tests, the comments can even be more distracting than. If you like to put them in that's all fine by me, but if the structure is evident I personally think they can be omitted.

[slarse]

When naming variables, try to describe their intent instead of their static type. For example, this is more informative:

Suggested change

	AbstractTemplate abstractTemplate = new AbstractTemplate() {
	AbstractTemplate zeroParameterTemplate = new AbstractTemplate() {

[Rohitesh-Kumar-Jain]

Avoid using raw types.

Thanks for pointing this, I will keep this in mind for future PRs as well.

I think I have made the required changes

[slarse]

@Rohitesh-Kumar-Jain Excellent work, will merge!

@monperrus Quick question: the rule about leaving pull requests open for at least one day, is that after being approved for merge, or from the time of the PR being opened? I interpret the guidelines as the latter, but just want to make sure :)

[monperrus]

Quick question: the rule about leaving pull requests open for at least one day, is that after being approved for merge, or from the time of the PR being opened?

From the time of the PR being opened.

[slarse]

@monperrus Okay, thanks.

Now however, all checks are green yet I can't merge the PR (says I'm not authorized). Could you have a look at the permissions?

[monperrus]

The reason is that we're using "Restrict who can push to matching branches" to "Organization administrators, repository administrators, and users with the Maintain role. "

This means that write access was not enough. Given you the "maintainer" role.

Could you try again?

[slarse]

All good, first merge done :)

[monperrus]

yeah!