Skip to content

More unit test improvements #682

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

Merged
merged 1 commit into from
Feb 11, 2023
Merged

Conversation

kingthorin
Copy link
Collaborator

  • BaseFakerTest.java > Allow callers to optionally specify a regex to match against.
  • UnitTest classes > Use newer unit test infrastructure where practical.

Signed-off-by: kingthorin kingthorin@users.noreply.github.com

@kingthorin
Copy link
Collaborator Author

kingthorin commented Feb 10, 2023

I plan to continue through this, just wanted to submit it in case: 1) Anyone hated how I'd changed things. 2) You'd know it was being worked on 😉

This isn't a blocker for 1.8.0.

@kingthorin kingthorin force-pushed the moar-tests branch 2 times, most recently from d2453f5 to 31caefc Compare February 10, 2023 18:56
@codecov-commenter
Copy link

Codecov Report

Merging #682 (d2453f5) into main (4707012) will decrease coverage by 0.04%.
The diff coverage is n/a.

❗ Current head d2453f5 differs from pull request most recent head 31caefc. Consider uploading reports for the commit 31caefc to get more accurate results

📣 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     #682      +/-   ##
============================================
- Coverage     92.64%   92.61%   -0.04%     
+ Complexity     2624     2610      -14     
============================================
  Files           284      283       -1     
  Lines          5356     5319      -37     
  Branches        581      574       -7     
============================================
- Hits           4962     4926      -36     
  Misses          248      248              
+ Partials        146      145       -1     
Impacted Files Coverage Δ
...ker/idnumbers/pt/br/IdNumberGeneratorPtBrUtil.java 92.59% <0.00%> (-3.71%) ⬇️
src/main/java/net/datafaker/formats/Yaml.java
.../datafaker/transformations/sql/SqlTransformer.java 85.92% <0.00%> (+0.72%) ⬆️

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

@kingthorin kingthorin force-pushed the moar-tests branch 2 times, most recently from e2d4cb2 to 3398998 Compare February 10, 2023 23:11
@kingthorin kingthorin changed the title More unit test improvements - WIP More unit test improvements Feb 10, 2023
@kingthorin
Copy link
Collaborator Author

No longer WIP, ready for review.

@snuyanzin
Copy link
Collaborator

I didn't get where regex is checked in new way?

@kingthorin kingthorin force-pushed the moar-tests branch 3 times, most recently from bc4f38f to 3a9bf31 Compare February 11, 2023 12:25
@kingthorin
Copy link
Collaborator Author

I didn't get where regex is checked in new way?

Ugh, thanks. I lost it along the way in some step of the implementation/re-implementation. Fixed now.

@kingthorin
Copy link
Collaborator Author

@snuyanzin I think I've addressed both your concerns. I put the Pattern.compile inside the regex assertion, let me know if you think it should be in the of method. Checking regex string isEmpty seemed easiest. But, I guess I could try/catch on PatternSyntaxException or check empty pattern ^$ and set it null, then null check it prior to the assertion if that's preferable.

- BaseFakerTest.java > Allow callers to optionally specify a
regex to match against.
- UnitTest classes > Use newer unit test infrastructure where practical.

Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
@kingthorin
Copy link
Collaborator Author

Fixed the build/test failure.

@kingthorin kingthorin merged commit a64eca3 into datafaker-net:main Feb 11, 2023
@kingthorin kingthorin deleted the moar-tests branch February 11, 2023 20:49
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.

3 participants