Skip to content

Make concurrent tests more stable#273

Merged
snuyanzin merged 1 commit into
datafaker-net:masterfrom
snuyanzin:passport
Aug 4, 2022
Merged

Make concurrent tests more stable#273
snuyanzin merged 1 commit into
datafaker-net:masterfrom
snuyanzin:passport

Conversation

@snuyanzin

@snuyanzin snuyanzin commented Aug 2, 2022

Copy link
Copy Markdown
Collaborator

The strange thing about passports is that it generates something strange and then in a loop tries to check if the result of generation is ok or not.
For instance, in net.datafaker.passportnumbers.ChPassportNumber#getValidCh it tries to generates something with regex [A-Z][0-9A-Z][0-9]{7} and then checks result against E\\d\\d{7} and G\\d{8}. It is much faster to generate based on expected data.

Update fix concurrent issue at https://github.com/datafaker-net/datafaker/runs/7640952734?check_suite_focus=true

@codecov-commenter

codecov-commenter commented Aug 2, 2022

Copy link
Copy Markdown

Codecov Report

Merging #273 (3fbf2e0) into master (f6e1238) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #273   +/-   ##
=========================================
  Coverage     94.63%   94.63%           
  Complexity     1891     1891           
=========================================
  Files           195      195           
  Lines          3819     3819           
  Branches        391      391           
=========================================
  Hits           3614     3614           
  Misses          104      104           
  Partials        101      101           
Impacted Files Coverage Δ
.../java/net/datafaker/service/FakeValuesService.java 87.92% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@bodiam

bodiam commented Aug 2, 2022

Copy link
Copy Markdown
Contributor

Hi @snuyanzin , I got rid of all that code here :

c0291fb

@snuyanzin

Copy link
Copy Markdown
Collaborator Author

@bodiam
ok, great.
However the funny thing is that the initial commit leads to test concurrent issue...

@bodiam

bodiam commented Aug 2, 2022

Copy link
Copy Markdown
Contributor

@snuyanzin I saw that indeed, but I didn't look into a solution for that yet. Seems it's still failing?

@snuyanzin snuyanzin changed the title Adopt regex to speed up generation Make concurrent tests more stable Aug 3, 2022
@snuyanzin

snuyanzin commented Aug 3, 2022

Copy link
Copy Markdown
Collaborator Author

At least now it seems more stable: I scheduled it 5 times in a row for the latest commit in this PR and they are still green

@snuyanzin
snuyanzin merged commit de6f146 into datafaker-net:master Aug 4, 2022
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