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

Creating multiple regular expression requests in rapid succession yields same result (.NET framework) #1183

Closed
MagnusMikkelsen opened this issue Jun 23, 2020 · 4 comments · Fixed by #1187

Comments

@MagnusMikkelsen
Copy link
Contributor

The test below (usually) fails in .NET framework fails in .NET framework, but not in core.

The underlying reason for this is that Xeger is used in autofixture without injecting an object of type Random, and Xeger uses new Random() - without injecting a seed.

The test passes, in .NET Core, because new Random() internally generates a unique seed.
However, as you probably know, in framework, it uses tick count as seed.

I haven't created a pr, because I didn't know where to do it to - does it make sense to create a pr with a failing test to master?

And I don't know if it should be fixed here in AF, or a pr should be created in the Fare repo.

What do you think? I'd like to fix this - but I'm not sure how to proceed...

        [Theory]
        [InlineData(@"^[A-Z]{27}$")]
        public void CreateTwoWithRegularExpressionRequestReturnsDifferentResults(string pattern)
        {
            // Arrange
            var sut = new RegularExpressionGenerator();
            
            var request = new RegularExpressionRequest(pattern);
            var dummyContext = new DelegatingSpecimenContext();
            // Act
            var result1 = sut.Create(request, dummyContext);
            var result2 = sut.Create(request, dummyContext);
            
            // Assert
            Assert.NotEqual(result1, result2);
        }
@moodmosaic
Copy link
Member

moodmosaic commented Jun 23, 2020

Thank you for reporting this 👍 It looks like the issue can be fixed in Fare, as per moodmosaic/Fare#26 (comment).

@MagnusMikkelsen
Copy link
Contributor Author

👍

@moodmosaic
Copy link
Member

Using a singleton-scoped (e.g. private readonly or perhaps private static readonly) Random instance supplied in

string regex = new Xeger(pattern).Generate();

as per this Xeger constructor overload should fix the issue.

@MagnusMikkelsen
Copy link
Contributor Author

Using a singleton-scoped (e.g. private readonly or perhaps private static readonly) Random instance...
... should fix the issue.

I think that's what I'm trying to do in #1187 ?

string regex = new Xeger(pattern, this.random).Generate();

zvirja pushed a commit that referenced this issue Jul 8, 2020
…identical strings (#1187)

* Proposed fix for #1183 - creating multiple regex within short time might yield identical stringsr #1183

Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

* Suggestions from codereview:
Added a test to FixtureTests, to verify that #1183 is fixed as suggested here: #1187 (comment)

This required making sure that the outcome space of the regex pattern is great enough. Xeger seems to favor shorter strings, and this gave collisions when generating RegularExpressionValidatedType . So minimum length of the pattern in this type was lengthened from 1 to 20.

Removed assertion roulette in RegularExpressionGeneratorTests.CreateMultipleWithRegularExpressionRequestReturnsDifferentResults(...)
according to suggestion: #1187 (comment)

Co-authored-by: Magnus Mikkelsen <mmi@kamstrup.dk>
Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>
Co-authored-by: MagnusMikkelsen <cactus1978@gmail.com>
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 a pull request may close this issue.

2 participants