Skip to content

Refactor1 : Worked on the failed build for previous PR #739#744

Merged
kingthorin merged 7 commits intodatafaker-net:mainfrom
almasfiza:refactor1
Mar 29, 2023
Merged

Refactor1 : Worked on the failed build for previous PR #739#744
kingthorin merged 7 commits intodatafaker-net:mainfrom
almasfiza:refactor1

Conversation

@almasfiza
Copy link
Contributor

The build was failing because of using the same function name for int and long generating functions. I have only added the code for int function - decomposed the conditional in refactoring, all the rest of the changes are as same as previous PR. (#739)

@what-the-diff
Copy link

what-the-diff bot commented Mar 29, 2023

PR Summary

  • Improved code readability in Barcode.java
    The method calculateVar was added to enhance readability and minimize code duplication.
  • Enhanced number validation in Number.java
    A new method, isValidRange(), was added for better readability of numberBetween(int min, int max).
  • Optimized string handling in FakerIDN
    StringBuilder is now used instead of concatenating strings in a loop, leading to increased efficiency.

@snuyanzin
Copy link
Collaborator

snuyanzin commented Mar 29, 2023

The build was failing because of using the same function name for int and long generating functions.

this does not look like true.
And by the way, I do not see your function for long numbers, instead there is something for float.
The exception from log files is

Stack trace (Click the triangle/control to the left to expand
2023-03-29T02:31:45.1222195Z 02:31:45.121 [�[1;31mERROR�[m] net.datafaker.providers.base.NumberTest.numberBetweenLongLongRandomizationQuality  Time elapsed: 0.018 s  <<< ERROR!
2023-03-29T02:31:45.1223167Z java.lang.RuntimeException: error in uniquePercentageOfResults
2023-03-29T02:31:45.1224394Z 	at net.datafaker.providers.base.NumberTest.uniquePercentageOfResults(NumberTest.java:330)
2023-03-29T02:31:45.1225230Z 	at net.datafaker.providers.base.NumberTest.lambda$numberBetweenLongLongRandomizationQuality$5(NumberTest.java:255)
2023-03-29T02:31:45.1226087Z 	at net.datafaker.providers.base.NumberTest.numberBetweenLongLongRandomizationQuality(NumberTest.java:263)
2023-03-29T02:31:45.1226918Z 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
2023-03-29T02:31:45.1227543Z 	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
2023-03-29T02:31:45.1228116Z 	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
2023-03-29T02:31:45.1228813Z 	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
2023-03-29T02:31:45.1229616Z 	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
2023-03-29T02:31:45.1230427Z 	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
2023-03-29T02:31:45.1231209Z 	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
2023-03-29T02:31:45.1232038Z 	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
2023-03-29T02:31:45.1232963Z 	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
2023-03-29T02:31:45.1234466Z 	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
2023-03-29T02:31:45.1235406Z 	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
2023-03-29T02:31:45.1236296Z 	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
2023-03-29T02:31:45.1237216Z 	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
2023-03-29T02:31:45.1238096Z 	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
2023-03-29T02:31:45.1238965Z 	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
2023-03-29T02:31:45.1239866Z 	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
2023-03-29T02:31:45.1240788Z 	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217)
2023-03-29T02:31:45.1241659Z 	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
2023-03-29T02:31:45.1242535Z 	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213)
2023-03-29T02:31:45.1243415Z 	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138)
2023-03-29T02:31:45.1244251Z 	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
2023-03-29T02:31:45.1245082Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
2023-03-29T02:31:45.1245915Z 	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
2023-03-29T02:31:45.1246751Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
2023-03-29T02:31:45.1247480Z 	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
2023-03-29T02:31:45.1248183Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
2023-03-29T02:31:45.1249001Z 	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
2023-03-29T02:31:45.1249854Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
2023-03-29T02:31:45.1250660Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
2023-03-29T02:31:45.1251684Z 	at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:202)
2023-03-29T02:31:45.1253151Z 	at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.executeNonConcurrentTasks(ForkJoinPoolHierarchicalTestExecutorService.java:172)
2023-03-29T02:31:45.1254999Z 	at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:152)
2023-03-29T02:31:45.1256094Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
2023-03-29T02:31:45.1256913Z 	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
2023-03-29T02:31:45.1257746Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
2023-03-29T02:31:45.1258462Z 	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
2023-03-29T02:31:45.1259179Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
2023-03-29T02:31:45.1260168Z 	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
2023-03-29T02:31:45.1261152Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
2023-03-29T02:31:45.1261963Z 	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
2023-03-29T02:31:45.1262991Z 	at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:202)
2023-03-29T02:31:45.1263908Z 	at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:194)
2023-03-29T02:31:45.1264471Z 	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
2023-03-29T02:31:45.1265051Z 	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
2023-03-29T02:31:45.1265623Z 	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
2023-03-29T02:31:45.1266189Z 	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
2023-03-29T02:31:45.1266804Z 	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)
2023-03-29T02:31:45.1267352Z Caused by: java.lang.IllegalArgumentException: bound must be positive
2023-03-29T02:31:45.1267874Z 	at net.datafaker.service.RandomService.nextLong(RandomService.java:50)
2023-03-29T02:31:45.1268426Z 	at net.datafaker.providers.base.Number.numberBetween(Number.java:79)
2023-03-29T02:31:45.1269106Z 	at net.datafaker.providers.base.NumberTest.lambda$numberBetweenLongLongRandomizationQuality$4(NumberTest.java:255)
2023-03-29T02:31:45.1269860Z 	at net.datafaker.providers.base.NumberTest.uniquePercentageOfResults(NumberTest.java:326)
2023-03-29T02:31:45.1270341Z 	... 49 more

@kingthorin
Copy link
Collaborator

The failure must be related to your Number.java changes.

2023-03-29T02:31:45.1267352Z Caused by: java.lang.IllegalArgumentException: bound must be positive
2023-03-29T02:31:45.1267874Z 	at net.datafaker.service.RandomService.nextLong(RandomService.java:50)
2023-03-29T02:31:45.1268426Z 	at net.datafaker.providers.base.Number.numberBetween(Number.java:79)
2023-03-29T02:31:45.1269106Z 	at net.datafaker.providers.base.NumberTest.lambda$numberBetweenLongLongRandomizationQuality$4(NumberTest.java:255)
2023-03-29T02:31:45.1269860Z 	at net.datafaker.providers.base.NumberTest.uniquePercentageOfResults(NumberTest.java:326)
2023-03-29T02:31:45.1270341Z 	... 49 more

@kingthorin
Copy link
Collaborator

@snuyanzin is the JavaDoc in Number.java wrong?

     * @param min the lower bound (include min)
     * @param max the lower bound (not include max)

Shouldn't max be "upper"?

@snuyanzin
Copy link
Collaborator

@snuyanzin is the JavaDoc in Number.java wrong?

yes, 3 times
good catch

@almasfiza
Copy link
Contributor Author

almasfiza commented Mar 29, 2023

The failure must be related to your Number.java changes.

Yes, thank you for pointing it out, fixed.

@almasfiza
Copy link
Contributor Author

@snuyanzin is the JavaDoc in Number.java wrong?

     * @param min the lower bound (include min)
     * @param max the lower bound (not include max)

Shouldn't max be "upper"?

yes! did not notice it at first, made the changes in java doc.

@almasfiza
Copy link
Contributor Author

@snuyanzin is the JavaDoc in Number.java wrong?

     * @param min the lower bound (include min)
     * @param max the lower bound (not include max)

Shouldn't max be "upper"?

fixed.

@kingthorin
Copy link
Collaborator

Thanks

@almasfiza almasfiza requested review from kingthorin and snuyanzin and removed request for snuyanzin March 29, 2023 14:11
@kingthorin
Copy link
Collaborator

They haven't posted an outage yet but it seems like something is up with GitHub ations.

@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2023

Codecov Report

Merging #744 (03042b6) into main (f4699de) will decrease coverage by 0.02%.
The diff coverage is 86.66%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main     #744      +/-   ##
============================================
- Coverage     92.84%   92.82%   -0.02%     
- Complexity     2592     2595       +3     
============================================
  Files           281      281              
  Lines          5126     5130       +4     
  Branches        530      530              
============================================
+ Hits           4759     4762       +3     
- Misses          240      241       +1     
  Partials        127      127              
Impacted Files Coverage Δ
...ain/java/net/datafaker/providers/base/Barcode.java 88.09% <60.00%> (-2.39%) ⬇️
...main/java/net/datafaker/providers/base/Number.java 100.00% <100.00%> (ø)
src/main/java/net/datafaker/service/FakerIDN.java 90.90% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kingthorin
Copy link
Collaborator

@almasfiza
Copy link
Contributor Author

https://www.githubstatus.com/incidents/z3c6q056q332

image
Should I work on something from my side for this? I tried doing maven test and compile on my local and it works.

@kingthorin
Copy link
Collaborator

I think you need to pickup these changes: #745

Assuming upstream is this repo, origin is your fork and you have the PR branch checked out:

git fetch upstream
git rebase upstream/main
git push --force-with-lease

@almasfiza
Copy link
Contributor Author

almasfiza commented Mar 29, 2023

image

I get this error.

@kingthorin
Copy link
Collaborator

Looks like you have origin set to this repo instead of yours.

@kingthorin
Copy link
Collaborator

Or maybe you aren't tracking the right repo. Try push -u origin refactor1 --force-with-lease if that fails then git remote -v and provide the output.

@almasfiza
Copy link
Contributor Author

almasfiza commented Mar 29, 2023

Or maybe you aren't tracking the right repo. Try push -u origin refactor1 --force-with-lease if that fails then git remote -v and provide the output.

Yes, i tried changing the origin to my repo, but now when i try fetch, it does not return anything.

image

Tried the push -u origin refactor1 --force-with-lease too
image

@kingthorin
Copy link
Collaborator

Okay, when you fetch now there's nothing to fetch because you've done it and things are up-to-date. Have you made local changes that haven't been pushed? What does git status say?

@almasfiza
Copy link
Contributor Author

almasfiza commented Mar 29, 2023

Okay, when you fetch now there's nothing to fetch because you've done it and things are up-to-date. Have you made local changes that haven't been pushed? What does git status say?

I have not made any changes. git status shows On branch refactor1 nothing to commit, working tree clean

I was trying to fetch the changes from #745.

@kingthorin
Copy link
Collaborator

Okay, try this then.

git fetch origin
# Shouldn't really do anything but maybe it'll sort out the stale situation
git fetch upstream
# Shouldn't really do anything because there hasn't been any changes in the last hour or so, but may as well ensure up-to-date
git rebase upstream/main
git push -u origin refactor1 --force

@kingthorin
Copy link
Collaborator

I was trying to fetch the changes from #745.

They were pretty minor, I'd suggest just re-implementing them after you get this sorted out.

@kingthorin
Copy link
Collaborator

kingthorin commented Mar 29, 2023

image

Yay, looks like it worked out that time 🎉

@almasfiza
Copy link
Contributor Author

Yay, looks like it worked out that time 🎉

Yes, it works! Hoping it passes all the checks. Thank you very much for your help and patience! :') Still figuring open source out.

@kingthorin
Copy link
Collaborator

No problem, there's a learning curve, been there 😉 all good.

@almasfiza almasfiza requested review from snuyanzin and removed request for kingthorin March 29, 2023 17:23
@snuyanzin
Copy link
Collaborator

thanks for your contribution

lgtm, once ci is passed

@almasfiza
Copy link
Contributor Author

thanks for your contribution

lgtm, once ci is passed

Thank you,

The checks are still queued - waiting to run this check.

@almasfiza
Copy link
Contributor Author

thanks for your contribution

lgtm, once ci is passed

All checks have passed.

@kingthorin kingthorin merged commit 17575b9 into datafaker-net:main Mar 29, 2023
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 this pull request may close these issues.

4 participants