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

Question about failing unit tests #14

Closed
JvanderStad opened this issue Feb 25, 2016 · 7 comments
Closed

Question about failing unit tests #14

JvanderStad opened this issue Feb 25, 2016 · 7 comments

Comments

@JvanderStad
Copy link
Contributor

Hi there,

in the locale files there is support for alternative combining of address strings. For example in the locale nl (Dutch) the cities can be combined using the following rules:

https://github.com/bchavez/Bogus/blob/master/Source/Bogus/data/nl.locale.json#L44

"city": [
        "#{Name.first_name}#{city_suffix}",
        "#{Name.last_name}#{city_suffix}",
        "#{city_prefix} #{Name.first_name}#{city_suffix}",
        "#{city_prefix} #{Name.last_name}#{city_suffix}"
      ]

Those rules are now hardcoded in https://github.com/bchavez/Bogus/blob/master/Source/Bogus/DataSets/Address.cs#L40

I have commited support for thore rules in the following commit:

JvanderStad@bfb57a0

However, some unittests are failing:

image
image

eg:

Expected string to be 
"1860 Bechtelar Rest" with a length of 19, but 
"60643 Oberbrunner Bypass" has a length of 24.

   at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message) in z:\Builds\work\eed5b735bfe0fb8d\Shared\Execution\LateBoundTestFramework.cs:line 30
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args) in z:\Builds\work\eed5b735bfe0fb8d\FluentAssertions.Core\Execution\AssertionScope.cs:line 197
   at FluentAssertions.Primitives.StringEqualityValidator.ValidateAgainstLengthDifferences() in z:\Builds\work\eed5b735bfe0fb8d\FluentAssertions.Core\Primitives\StringEqualityValidator.cs:line 40
   at FluentAssertions.Primitives.StringValidator.Validate() in z:\Builds\work\eed5b735bfe0fb8d\FluentAssertions.Core\Primitives\StringValidator.cs:line 42
   at FluentAssertions.Primitives.StringAssertions.Be(String expected, String because, Object[] reasonArgs) in z:\Builds\work\eed5b735bfe0fb8d\FluentAssertions.Core\Primitives\StringAssertions.cs:line 40
   at Bogus.Tests.AddressTest.can_get_a_street_address() in c:\Workspace\Ontwikkeling\GitHub\Bogus\Source\Bogus.Tests\AddressTest.cs:line 40

As far as I understand this is how the unit tests in this project work:

The seed is predetermined and every Seed.Next() will return predictable results ( ❔ )

Because I have changed the the lookup of string values, the amount of calls to Seed.Next() has changed, as a result the unit tests are failing.

That's my assumption or am I missing something?

@bchavez
Copy link
Owner

bchavez commented Feb 25, 2016

Hi @JvanderStad , right-o and on point, I think your assumption is correct. Seed is only predictable on the random values pulled from Seed.Next. So, if the number of calls to Next changes it can disrupt the unit tests expected output.

@JvanderStad
Copy link
Contributor Author

thanks 👍

I'll fix the tests, pr coming soon..

@bchavez
Copy link
Owner

bchavez commented Feb 25, 2016

You bring up a good point tough... If you checkout Person.cs, a person is being Initialized (and values pulled including address) when a Faker is constructed. The more correct way is to probably lazily delay person initialization this until a person is actually needed. I think this is what's goofing up the unit tests for you :(

@JvanderStad
Copy link
Contributor Author

I've created 2 new methods: GetRandomValue() and ParseTokens the ParseTokens is recursive. So the amount of calls to Seed.Next is changed by my commit.

Also, I made another branch with lazy loading the Person.. JvanderStad@6695892

@JvanderStad
Copy link
Contributor Author

pr #15 and pr #16

@bchavez
Copy link
Owner

bchavez commented Feb 25, 2016

SUPER cool! Thanks so much. I'll get a release out today for you.

@bchavez
Copy link
Owner

bchavez commented Feb 25, 2016

@JvanderStad You changes are live in v5.0.1 and v5.0.1-beta-2. Thanks again for your contribution! Should appear soon when nuget index catches up.

https://www.nuget.org/packages/Bogus/

Brian

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

No branches or pull requests

2 participants