No java util date rule test #5

FanJups · 2019-05-14T21:16:06Z

Summary

java.util.Date is deprecated so I decided to add a new rule NoJavaUtilDateRuleTest.java and suggested other libraries :

java.time ( java 8)
java.util.GregorianCalendar
java.Text.DateFormat

Details

I created a new rule file src/main/java/ com.societegenerale.commons.plugin.rules.NoJavaUtilDateRuleTest.java and added a new String constant (NO_JAVA_UTIL_DATE_VIOLATION_MESSAGE) in src/main/java/com.societegenerale.commons.plugin.utils.ArchUtils.java
I created src/test/java/com.societegenerale.commons.plugin.rules.NoJavaUtilDateRuleTestTest.java to test the rule and changed NumberOfRulesAvailable from 9 to 10 & totalClassesInPathWhenPackageFolderDoesNotExists from 12 to 13 in src/test/java/com.societegenerale.commons.plugin.rules.ArchUtilsTest.java
I created 3 files ObjectWithJavaTextDateFormat.java ; ObjectWithJavaUtilDateReferences.java and ObjectWithJavaUtilGregorianCalendar.java in src/test/java/com.societegenerale.commons.plugin.rules.classesForTests
I added my email in pom.xml

Context

java.util.Date library is deprecated
Use Java8 java.time or java.util.GregorianCalendar or java.text.DateFormat to parse and format dates instead of java.util.Date library because they support internationalization better

Checklist:

[x ] I've added tests for my code.
[x ] Documentation has been updated accordingly to this PR

Related issue :

Syncing my fork to the original repository

…coveralls analysis

vincent-fuchs

Hi, thanks a lot for your first contribution to our project !!! It's much appreciated !!

please have a look at the comments I've provided. Also, please check the code formatting - a lot of the lines changed are actually only some formatting changes. We'll probably have to provide an automated code style check

vincent-fuchs · 2019-05-15T09:19:28Z

src/main/java/com/societegenerale/commons/plugin/rules/NoJavaUtilDateRuleTest.java

+							ArchUtils.NO_JAVA_UTIL_DATE_VIOLATION_MESSAGE + " - class: "
+									+ methodCall.getOriginOwner().getName() + " - line: "
+									+ methodCall.getLineNumber()));
+				}


code from lines 60-71 is not unit tested properly : I've commented it locally, and the tests are still passing. so please add a test for that use case, or remove that part of the code

It's a pleasure to contribute to this project. The source code format is automatically defined by Eclipse. I will also focus on these tests. Thanks for your idea.

vincent-fuchs · 2019-05-15T16:47:52Z

src/main/java/com/societegenerale/commons/plugin/rules/NoJavaUtilDateRuleTest.java

+				List<JavaField> classesWithJavaUtilDateFields = item.getAllFields().stream()
+						.filter(field -> isJavaUtilDateField(field)).collect(toList());
+
+				for (JavaField field : classesWithJavaUtilDateFields) {


why do it in 2 steps ? instead of collecting the items, you could directly a foreach and add the violations in events

vincent-fuchs · 2019-05-15T16:57:26Z

src/test/java/com/societegenerale/commons/plugin/rules/NoJavaUtilDateRuleTestTest.java

+	}
+
+	@Test(expected = Throwable.class)
+	public void shouldThrowNoJavaUtilDateViolation() {


what's the added value of that test compared to previous one ? It's same, no ?

You are right, I'll delete this second test.

I made some changes on NoJavaUtilDateRuleTest.java and NoJavaUtilDateRuleTestTest.java about the tests and code style.

FanJups · 2019-05-15T21:39:35Z

Thank you very much @vincent-fuchs , what do you think about the changes I've made ?

vincent-fuchs · 2019-05-19T16:07:47Z

src/main/java/com/societegenerale/commons/plugin/rules/NoJavaUtilDateRuleTest.java

+						});
+			}
+
+			@SuppressWarnings("deprecation")


remove the SuppressWarnings annotation and modify your code so that no deprecation warning comes up.

vincent-fuchs · 2019-05-19T16:13:01Z

Thank you very much @vincent-fuchs , what do you think about the changes I've made ?

Thanks for the changes, but there is still one thing I don't get :

I still don't see a test for

arch-unit-maven-plugin/src/main/java/com/societegenerale/commons/plugin/rules/NoJavaUtilDateRuleTest.java

Lines 54 to 61 in 1051de7

    
           item.getCodeUnits().stream().filter(codeUnit -> codeUnit instanceof JavaMethod) 
        
           		.flatMap(method -> method.getMethodCallsFromSelf().stream()) 
        
           		.filter(method -> method instanceof JavaMethodCall) 
        
           		.filter(method -> isMethodUsingJavaUtilDateInternally(method)).forEach(methodCall -> { 
        
           			events.add(SimpleConditionEvent.violated(methodCall.getOriginOwner(), 
        
           					ArchUtils.NO_JAVA_UTIL_DATE_VIOLATION_MESSAGE + " - class: " 
        
           							+ methodCall.getOriginOwner().getName())); 
        
           		});

. I can still remove them, and no test is failing.

Also :

I've added a comment : https://github.com/societe-generale/arch-unit-maven-plugin/pull/5/files#r285387079
I've pushed a simplified version of NoJavaUtilDateRuleTestTest.java in the branch
we've just pushed some fairly big changes in master : your branch needs to be rebased and conflicts fixed.

…TestTest.java / ArchUtilsTest.java / ObjectWithAdateField.java / ObjectWithJavaUtilDateReferences.java NoJavaUtilDateRuleTest.java : 1) No method is imported using the java.util.Date library so I deleted code related to the cheking of imported methods 2) I created a constant NO_JAVA_UTIL_DATE_VIOLATION_MESSAGE 3) I used SRC_CLASSES_FOLDER from ArchRuleTest NoJavaUtilDateRuleTestTest.java : I deleted ObjectWithJavaUtilDateReferences.java and created ObjectWithAdateField.java ArchUtilsTest.java : changed NumberOfRulesAvailable from 9 to 10 & totalClassesInPathWhenPackageFolderDoesNotExists from 16 to 17

FanJups · 2019-05-20T14:15:18Z

@vincent-fuchs I applied new changes related to : NoJavaUtilDateRuleTest.java / NoJavaUtilDateRuleTestTest.java / ArchUtilsTest.java / ObjectWithAdateField.java / ObjectWithJavaUtilDateReferences.java

NoJavaUtilDateRuleTest.java :

No method is imported using the java.util.Date library so I deleted code related to the cheking of imported methods
I created a constant NO_JAVA_UTIL_DATE_VIOLATION_MESSAGE
I used SRC_CLASSES_FOLDER from ArchRuleTest
SuppressWarnings annotation is deleted

NoJavaUtilDateRuleTestTest.java : I deleted ObjectWithJavaUtilDateReferences.java and created ObjectWithAdateField.java

ArchUtilsTest.java : I changed NumberOfRulesAvailable from 9 to 10 & totalClassesInPathWhenPackageFolderDoesNotExists from 16 to 17

vincent-fuchs · 2019-05-21T11:15:06Z

Thanks @FanJups ! We'll do a release soon, you'll be able to use it directly

FanJups · 2019-05-21T11:22:01Z

Thanks @vincent-fuchs , that's great

New rule added : no java util date (#5)

adding "Official maintainers" section

FanJups added 3 commits May 14, 2019 11:45

Merge pull request #1 from societe-generale/master

fd147a0

Syncing my fork to the original repository

No Java Util Date Rule Test

2804bb9

No Java Util Date Rule Test : I added more accurate tests related to …

179c700

…coveralls analysis

vincent-fuchs reviewed May 15, 2019

View reviewed changes

Code improvement

1051de7

I made some changes on NoJavaUtilDateRuleTest.java and NoJavaUtilDateRuleTestTest.java about the tests and code style.

vincent-fuchs reviewed May 19, 2019

View reviewed changes

refactoring NoJavaUtilDateRuleTestTest

3cf346d

FanJups added 2 commits May 20, 2019 13:11

Merge branch 'master' into no-java-util-date-rule-test

644d9bd

vincent-fuchs merged commit fb6183b into societe-generale:master May 21, 2019

FanJups referenced this pull request in FanJups/arch-unit-maven-plugin May 22, 2019

Merge pull request #3 from societe-generale/master

6d93916

New rule added : no java util date (#5)

FanJups deleted the no-java-util-date-rule-test branch May 28, 2019 12:54

vincent-fuchs pushed a commit that referenced this pull request Jul 31, 2019

Merge pull request #5 from societe-generale/master

d185aa7

adding "Official maintainers" section

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No java util date rule test #5

No java util date rule test #5

FanJups commented May 14, 2019

vincent-fuchs left a comment

vincent-fuchs May 15, 2019

FanJups May 15, 2019

vincent-fuchs May 15, 2019

FanJups May 15, 2019

vincent-fuchs May 15, 2019

FanJups May 15, 2019

FanJups commented May 15, 2019 •

edited

Loading

vincent-fuchs May 19, 2019

vincent-fuchs commented May 19, 2019

FanJups commented May 20, 2019

vincent-fuchs commented May 21, 2019

FanJups commented May 21, 2019

No java util date rule test #5

No java util date rule test #5

Conversation

FanJups commented May 14, 2019

Summary

Details

Context

Checklist:

Related issue :

vincent-fuchs left a comment

Choose a reason for hiding this comment

vincent-fuchs May 15, 2019

Choose a reason for hiding this comment

FanJups May 15, 2019

Choose a reason for hiding this comment

vincent-fuchs May 15, 2019

Choose a reason for hiding this comment

FanJups May 15, 2019

Choose a reason for hiding this comment

vincent-fuchs May 15, 2019

Choose a reason for hiding this comment

FanJups May 15, 2019

Choose a reason for hiding this comment

FanJups commented May 15, 2019 • edited Loading

vincent-fuchs May 19, 2019

Choose a reason for hiding this comment

vincent-fuchs commented May 19, 2019

FanJups commented May 20, 2019

vincent-fuchs commented May 21, 2019

FanJups commented May 21, 2019

FanJups commented May 15, 2019 •

edited

Loading