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

Add uniqueness to certain random values #333

Closed
rijkvanzanten opened this issue Dec 13, 2022 · 7 comments
Closed

Add uniqueness to certain random values #333

rijkvanzanten opened this issue Dec 13, 2022 · 7 comments

Comments

@rijkvanzanten
Copy link

rijkvanzanten commented Dec 13, 2022

Is this a regression?

No

Description

Heya! I'm using Falso in unit tests to generate randomized, yet plausible, data, but am sometimes faced with false negatives due to some methods not returning data that's reliably unique. For example if you use randFilePath, there's a chance that it'll return the same path multiple times as it just randomly picks from a fixed list: https://github.com/ngneat/falso/blob/main/packages/falso/src/lib/file-path.json

Would it be worth adding some (light) randomization to methods like this? For example, a random file-path could add a random hash at the end, so you'd end up with (assumed 'duplicate' output):

/usr/src/realbox-62d6d23.gif
/usr/src/realbox-51bd2b9.gif

(Similarly, when talking about file-path specifically, I think it'd be nice if randFilePath() would be a shortcut for randDirectoryPath() + randFileName(), and randFileName() using randFileExt() instead of defaulting to .pdf, but I digress. Happy to open a new issue for that tweak if you'd like to keep these two thoughts separate).

Please provide a minimal reproduction of the bug

Lets say you're in a unit test and mocking an external dependency that modifies a file path, for example:

import { expect, vi, test } from 'vitest';
import { join } from 'node:path';
import { randFilePath } from '@ngneat/falso'

vi.mock(join);

const myFunction = (input) => join('/root', input);

test('Uses join to prefix root to input', () => {
  const mockJoinOutput = randFilePath();
  vi.mocked(join).mockReturnValue(mockJoinOutput);
  
  const input = randFilePath();

  const output = myFunction(input);

  expect(join).toHaveBeenCalledWith('/root', input);
  expect(output).toBe(mockJoinOutput);
});

there's a decent chance that input and mockJoinOutput are identical, which causes the test to fail sporadically. Ensuring that the output of randFilePath() is unique—or at least more random—prevents that issue.

Please provide the environment you discovered this bug in

Node.js v18.12.1

Do you want to create a pull request?

Yes

@NetanelBasal
Copy link
Member

Any reason you're not using seed?

https://github.com/ngneat/falso#setting-a-randomness-seed

@rijkvanzanten
Copy link
Author

@NetanelBasal I don't think that fully solves this particular issue does it? It allows repeat tests to get the same values back every time, but it doesn't insure that those individual values are unique from each other in the first place. If I were to rely on seeds as a solution for this, I'd have to go through a number of seeds first to find one that returns unique values, and then use that, whereas I'm hoping randFilePath() could return a unique path regardless of seed used.

@NetanelBasal
Copy link
Member

Can you create a small stackblitz that explains your point, please?

@rijkvanzanten
Copy link
Author

The smallest explanation I have is:

import { randFilePath } from '@ngneat/falso';

const path1 = randFilePath();
const path2 = randFilePath();

path1 === path2; // This is sometimes true, should always be false

@PumpedSardines
Copy link
Contributor

Not sure I agree that randFilePath always should be different, randomness can be the same sometimes. However randFilePath is rather lacking in the amount of return values. There should definitely be a less than 1 in a 100 chance to get the same output.

For your particular case I would just use seed like @NetanelBasal suggested, however I do realise it's unsatisfying.

The question of uniqueness was discussed briefly in #220, but the solution there seemed rather strange imho.

@NetanelBasal
Copy link
Member

Agree with @PumpedSardines that randomness can be the same sometimes.

@rijkvanzanten
Copy link
Author

OK! Whatever you guys prefer. For my use cases, the high chance of collision is a big hindrance, so I'll rely on an alternative approach 👍🏻

@rijkvanzanten rijkvanzanten closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 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

No branches or pull requests

3 participants