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

Add ConstructorAssert instead of using ClassAssert #1871

Open
wants to merge 15 commits into
base: 3.x
Choose a base branch
from

Conversation

phx1999
Copy link

@phx1999 phx1999 commented May 11, 2020

Check List:

@joel-costigliola joel-costigliola changed the base branch from master to main June 21, 2020 05:32
}

/**
* Verifies that the actual {@code Constructor} is public (has {@code public} modifier).
Copy link
Contributor

Choose a reason for hiding this comment

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

This assert is testing for private modifier

Suggested change
* Verifies that the actual {@code Constructor} is public (has {@code public} modifier).
* Verifies that the actual {@code Constructor} is private (has {@code private} modifier).

}

/**
* Verifies that the actual {@code Constructor} is public (has {@code public} modifier).
Copy link
Contributor

Choose a reason for hiding this comment

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

This assert is testing for protected modifier

Suggested change
* Verifies that the actual {@code Constructor} is public (has {@code public} modifier).
* Verifies that the actual {@code Constructor} is protected (has {@code protected} modifier).

* }
*
* // these assertions succeed:
* assertThat(String.class.getDeclaredConstructor()).isPrivate();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* assertThat(String.class.getDeclaredConstructor()).isPrivate();
* assertThat(MyClass.class.getDeclaredConstructor()).isPrivate();

* }
*
* // these assertions succeed:
* assertThat(String.class.getDeclaredConstructor()).isProtected();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* assertThat(String.class.getDeclaredConstructor()).isProtected();
* assertThat(MyClass.class.getDeclaredConstructor()).isProtected();

}

/**
* Verifies that the actual {@code Constructor} is public (has {@code public} modifier).
Copy link
Contributor

Choose a reason for hiding this comment

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

This description does not describe the real checks

@@ -345,10 +345,11 @@ public SELF hasSameSizeAs(Iterable<?> other) {
*/
@Override
public SELF contains(@SuppressWarnings("unchecked") ELEMENT... values) {
arrays.assertContains(info, actual, values);
arrays.assertContains(info, actual, values);
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra padding

return myself;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Extra padding

@@ -3160,4 +3161,7 @@ private SELF newObjectArrayAssert(List<ELEMENT> filteredList) {
return filteredList.toArray(actualCopy);
}

// public boolean hasPublicConstructor(List<ELEMENT> parameters){
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't include not implemented parts in PR

}



Copy link
Contributor

Choose a reason for hiding this comment

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

Extra padding

}



Copy link
Contributor

Choose a reason for hiding this comment

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

Extra padding

@@ -586,6 +587,13 @@ public static ClassAssert assertThat(Class<?> actual) {
return new ThrowableAssert(actual);
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Please add JavaDoc

@@ -586,6 +587,13 @@ public static ClassAssert assertThat(Class<?> actual) {
return new ThrowableAssert(actual);
}


public static ConstructorAssert assertThat(Constructor actual){
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add space before {

* @param actual the actual value.
* @return the created assumption for assertion object.
*/
public static AbstractConstructorAssert<ConstructorAssert,Constructor> assumeThat(Constructor actual) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add space after ,

@joel-costigliola
Copy link
Member

@phx1999 can you remove all the xml files from the PR and address @valery1707 comments? thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: not ready A change that requires additional work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ConstructorAssert instead of using ClassAssert
3 participants