Fix broken random.alphaNumeric() test

[ST-DDT]

faker/test/random.spec.ts

Lines 180 to 187 in 431c108

	it('should be able to ban some characters', () => {
	const alphaText = faker.random.alphaNumeric(5, {
	bannedChars: ['a', 'p'],
	});
	expect(alphaText).toHaveLength(5);
	expect(alphaText).match(/[b-oq-z]/);
	});

faker/src/random.ts

Line 383 in 431c108

alphaNumeric(

It just has a very low chance of failing (~0.2%).

The test should be fixed like this:

- expect(alphaText).match(/[b-oq-z]/);
+ expect(alphaText).match(/^[0-9b-oq-z]{5}$/);

The same is true for the following test.
I also recommend to ban more chars, as the current test only has a chance of about ~25%(?) to actually the test the expected result.

Originally posted by @ST-DDT in #508 (comment)

[ST-DDT]

@Shinigami92 Do you want to fix it, or is it up for grabs?

[xDivisionByZerox]

I will do this. No problem.

[xDivisionByZerox]

Wouldn't it be easier to iterate over each banned character and expect the alpha text to not include it? Like this

it('should be able to ban some characters', () => {
  const bannedChars = ['a', 'p'];
  const alphaText = faker.random.alphaNumeric(5, {
    bannedChars,
  });

  expect(alphaText).toHaveLength(5);
  for(const bannedChar of bannedChars) {
    expect(alphaText.includes(bannedChar)).toBe(false);
  }
});

This would allow changing the bannedChars Array without the need to rewrite the RegEx.

[ST-DDT]

IMO we should ban all characters a - p to have a higher probability to "trigger" a banned character.

As for "foreach vs regex": I think it's fine either way.

@Shinigami92 Any preference?

[xDivisionByZerox]

We could also force test this by splitting this into 2 tests. One with all alphabetic characters banned and one with all numbers characters baned.

[ST-DDT]

I really like that idea.

foreach vs regex:
If it isn't too much work, start with both variants after each other (in the same method).
That makes comparing it easier.
Then we can decide on one of them easily afterwards.

[xDivisionByZerox]

Should the current test with some characters baned (instead of all alpha/number) still be reworked or replaced by the 2 tests I suggested?

Furthermore, what happens if a bannedChars Array with all alpha-numeric characters is provided? Should a test for that exist as well?

[xDivisionByZerox]

foreach vs regex:
If it isn't too much work, start with both variants after each other (in the same method).
That makes comparing it easier.
Then we can decide on one of them easily afterwards.

Do you mean 2 tests:

• one for banned alpha, one for banned numbers where each test matches against regex AND foreach
  OR
• one with regex matching against banned alpha AND numbers and one with foreach matching against banned alpha AND numbers

[ST-DDT]

The first one.

one for banned alpha, one for banned numbers where each test matches against regex AND foreach

[ST-DDT]

Furthermore, what happens if a bannedChars Array with all alpha-numeric characters is provided? Should a test for that exist as well?

Yes. Although I assume, that this will reveal another "bug", that we can only fix in v6.1. If that's the case disable it with it.todo().