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

test: improve tests for NameFilter class. #3975

Merged
merged 8 commits into from
Jun 11, 2021
35 changes: 34 additions & 1 deletion src/test/java/spoon/test/filters/FilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,46 @@ public void setup() throws Exception {

@Test
public void testNameFilter() throws Exception {
// contract: legacy NameFilter is tested and works
// contract: NameFilter finds the expected amount of elements when filtering
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good!


CtClass<?> foo = factory.Package().get("spoon.test.filters.testclasses").getType("Foo");
assertEquals("Foo", foo.getSimpleName());
Rohitesh-Kumar-Jain marked this conversation as resolved.
Show resolved Hide resolved
List<CtNamedElement> elements = foo.getElements(new NameFilter<>("i"));
assertEquals(1, elements.size());
}

@Test
void testNameFilterMatchesReturnsTrueForMatchingName() {
// contract: NameFilter.matches should return true for an element with a matching name

// arrange
String name = "someNiceName";
NameFilter<CtNamedElement> nameFilter = new NameFilter<>(name);
CtNamedElement elemWithMatchingName = new Launcher().getFactory().createLocalVariable();
elemWithMatchingName.setSimpleName(name);

// act
boolean matches = nameFilter.matches(elemWithMatchingName);

// assert
assertTrue(matches);
}
Rohitesh-Kumar-Jain marked this conversation as resolved.
Show resolved Hide resolved

@Test
public void testNameFilterThrowsExceptionForNull() {
// contract: NameFilter constructor should throw IllegalArgumentException when passed null

assertThrows(IllegalArgumentException.class, () -> new NameFilter<>(null));
}

@Test
public void testNameFilterGetType() {
// contract: getType method returns the datatype that the NameFilter class returns
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm truly sorry for going on and on here, but this contract still doesn't make sense to me. It says that a method returns the datatype that a class returns. But a class does not return anything, and so it does not make sense.

It's very important to be able to succinctly specify what a test is supposed to verify, so we need to keep iterating on this until you've nailed it. Feel free to just comment in this comment with suggestions before committing to something.

Copy link
Contributor Author

@Rohitesh-Kumar-Jain Rohitesh-Kumar-Jain Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm truly sorry for going on and on here

no no, it's my fault that iterations are still going on, had I raised a perfect PR, no iterations would have occurred. I will try to come with a succinct contract.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to go with the one suggested by you only // contract: NameFilter.getType() should return the CtNamedElement class


NameFilter<CtNamedElement> nameFilter = new NameFilter<>("i");
assertEquals(CtNamedElement.class, nameFilter.getType());
}

@Test
public void testFilters() {
CtClass<?> foo = factory.Package().get("spoon.test.filters.testclasses").getType("Foo");
Expand Down