faker-js / faker

Generate massive amounts of fake data in the browser and node.js
https://fakerjs.dev
Other
12.65k stars 910 forks source link

CI failure in git.commitEntry test #1803

Closed ST-DDT closed 1 year ago

ST-DDT commented 1 year ago

VITEST_SEQUENCE_SEED 1675266128640

grafik

https://github.com/faker-js/faker/actions/runs/4066068064/jobs/7002435580

The regex needs to be checked for correctness (blindy adding ' is a non goal).

ST-DDT commented 1 year ago

Also:

VITEST_SEQUENCE_SEED 1675271531940 grafik https://github.com/faker-js/faker/actions/runs/4066878355/jobs/7003536672

ST-DDT commented 1 year ago

The same error again:

VITEST_SEQUENCE_SEED 1675275945101 grafik

https://github.com/faker-js/faker/actions/runs/4067493397/jobs/7004904658

matthewmayer commented 1 year ago

the name comes from faker.person.fullName, which in the English locale can contain ' in addition to A-Za-z.

Whereas the regex only allows \w (which doesn't include '), . and space.

So blindly adding ' would indeed fix it.

If the same test was run in other locales, it would still fail as they could contain other unicode chars. But as the test is only run in English at the moment i think this would suffice.

matthewmayer commented 1 year ago

btw is there any way in the logs to make it print out the full string for expected value? The way it ellipsizes the full value is annoying.

Shinigami92 commented 1 year ago

btw is there any way in the logs to make it print out the full string for expected value? The way it ellipsizes the full value is annoying.

Vitest also has a documentation website, even with search function https://vitest.dev/config/#outputtruncatelength


Now the funny part: the option does not work 🤦 I will open an issue in Vitest...

ST-DDT commented 1 year ago

So blindly adding ' would indeed fix it.

I would still like to know which characters are allowed there (and how it is called).

Shinigami92 commented 1 year ago

https://github.com/vitest-dev/vitest/issues/2791

xDivisionByZerox commented 1 year ago

We could see the failing test as an implementation error as well. The questions we have to answer are:

If the same test was run in other locales, it would still fail as they could contain other unicode chars.

This could be solved by defining that the generated user is always a username instead of using a fullname.

matthewmayer commented 1 year ago

Unicode characters are permissible according to https://stackoverflow.com/questions/24845258/is-it-ok-to-use-a-long-non-ascii-name-for-the-user-name-git-configuration

matthewmayer commented 1 year ago

Honestly I'd just add the ' for now to fix the intermittent CI errors and leave a TODO to revisit this later if tests are added for every locale. Regex for Unicode text can be quite tricky, and I don't think this is a heavily used method in faker.

Shinigami92 commented 1 year ago
vite repo `git log --pretty=format:%an | sort -u` 👀 ``` ${Mr.DJA} 07akioni 0xflotus 4quar1usnow Aaron Bassett Aaron Cordova Aaron Gong Aayush Kapoor Abdellah Alaoui Solaimani Agustin Gomez Ahab Akito Tabira Alec Larson Alex Kozack Alex Lambiris Alexander Belov Alexander Raihelgaus Alexander von Weiss Alexandre Alexandre Bonaventure Geissmann Alexandre Hitchcox Alexey Shamrin Alexis Tyler Aleš Vaupotič Amit Gurbani Amorites Amour1688 Anbraten Andoni Abedul Andreas Girgensohn Andrejs Abrickis Andrew Andrew Powers Andrew Welch Andrey Trebler Andy Li Angel Gomez Ant Ante Sepic Anthony Campolo Anthony Catel Anthony Fu Arnaud Barré Arnaud Thomas D Aron Griffis Artem Khodyush Artur Ash Go Ashish Shubham Asko Kauppi Autumn Meadow Axe Azat S Bart Louwers Barthélémy Ledoux Bartosz Gościński Bechir Ba Ben Ben Halpern Ben Holmes Ben McCann Benedikt Allendorf BenoitRanque Bernardo Sunderhus Berton Zhu Bjorn Lu Blake Newman Bogdan Chadkin Borkesz Máté Bozhen Zheng Brett Pappas Bri Brice BERNARD Bálint Szekeres CGQAQ CHOYSEN CRIMX CSY54 Caleb Eby Candy Carlos Morales Carlos Rodrigues Carter Snook Cassio Zen Cedric Nicoloso Cesar Gomez Charles C Chris Chris Santamaria Christoph Nakazawa Christoph Werner Christopher Shank Claire Clay Clement Fradet Normand Cláudio Medina Costa Alexoglou Cristian Pallarés Curran Kelleher Cyrille Bourgois Cédric Eberhardt Daiki Ojima Daiz Dalibor Gogic Damian Stasik Dan Dan Jutan Daniel Imfeld Daniel Roe Daniel Schmelz Danny Feliz Dany Castillo Darin Cassler Darren Jennings David Bohn David Bonnet David Jackson David R. Myers David Öhlin Day Deisling Eduard Dimitris-Toulis Domantas Dominik G DouglasDev Drew Powers Dunqing Dustin Ryerson Dylan Piercey EGOIST Eder Soares Eduardo San Martin Morote Edward Sanders Eirik Sletteberg Em Zhan Enzo Innocenzi Eren Bets Eric Eastwood Eric Jolibois Espen Hovlandsdal Etienne Eugene Kopich Eugene Kopich ⚡ Evan Boehs Evan You Fansy Faris Ansari Fatih Aygün Ferenc Tamás Fernanda Sales Bittencourt de Lemos Florian Dreier Flyme Forzen FIsh Fran Dios Francois Valdy François Risoud François Wouts Fred K. Schott GaryNg Gautier Ben Aïm GavinRay97 Georgiy Bukharov Glen Huang Glen Little Gray Zhang Greg Leppert GrygrFlzr Guanghui Cheng Guille Guillermo Rauch HChange HG Hang Liang Han(ハン) Haoqun Jiang Happydev Harry Brundage Harry Porter-Mills Harsh Pandey Haruki Murasaki Hazlank HcySunYang HelKyle Henry Henry Vogt Hermann Stanew Herrington Darkholme Hex Homyee King Hugo ATTAL Hugo Alliaume Hung Viet Nguyen Hydrogen IU Ian Purvis Ian VanSchooten Ian Wright Ilya Artamonov Iron Lu Isaac Levy Israel Roldan Ivan Demchuk Ivan Sieder J Bruni JA Jack Chu Jack Franklin Jambon James Garbutt James George James Herdman James Lee James Tucker Jamie Spittal Janis Pritzkau Jasper Van Gestel Jay JayFate Jeff Yang Jeff Yang (Nay Thu Ya Aung) Jen Looper Jesse Jessica Sachs Jeudi Prando Araujo Jian Zhang Jiby Jose Joaquín Sánchez Joe Busillo Joel Ellis Joel Kuzmarski Johan Bergström Johannes Johannes Maas Johannes Schickling John Simons Johnson Chu JokcyLou Jonas Jonas Galvez Jonathan Herman Jordan Pittman Josef Brandl Josh Field Josh Larson Joshua Davidson JounQin Jovi De Croock JserWang Juan Martín Seery Julien Papini Jun Shindo Junwen Huang Justin Jérémie Cotiniaux Kael Kallyn Gowdy Kazuma Oe Kenichi Kamiya Kerim Hudson Kevin Hazy Kevin LEVRON Kia King Ishii Kid KonpekiCode Kristoffer K Kyle Herock Lee Lee Robinson Li Chen Li Zhequ Lijianan Lil-C0der Liu Bowen LocTran016 Lubomír Blažek Lucas Garron Lucas Mendelowski Ludovico Fischer Lukas Luuk de Vlieger MaMrEzO Maarten de Graaf Manan Tank Manu MA Marcel Lindig Marco Schumacher Mark Mark Ladyshau Markus Lanthaler Martin Kirilov Marvin Hagemeister Marvin Rudolph Mat Mathieu Mure Matia Matias Matias Capeletto Matt Nathan Matthew Phillips Matthew Runyon Matěj Volf Maurício Kishi Meet Zaveri Menci Mestery Michael Kurze Michael Oliver Michael P Smith Michael Warner Michel EDIGHOFFER Mike Ledger Minseok Suh Modder Me Máximo Mussini NONAME Naeemo Naitik Shah Naoki Endoh Nate Moore Natrim Nick Santos Nicolas Hedger Nihal Gonsalves Niputi Nurettin Kaya Nurul Huda (Apon) OBE Rocks! Obinna Ekwuno Oliver Zhou (毓杰) OliverTsang Olyno OmegaVesko OneNail Ophir LOJKINE Oreki S.H Ostap B Oyster Lee Patrick Dobbs Paulius Varna Pavel Zhukov Pedro Di Martino Peng Xiao Pengsha Ying Percy Ma Pete Nykänen Peter Chibisov Peter Mekhaeil Phil Chen Phinome PiEgg Pieter Pig Fang Pony Yan Prana Adiwira QiChang Li Rahul Kadyan Ramnath Shenoy Razio Razvan Stoenescu Reed Jones Remus Mate Renovate Bot Rich Harris Richard Petersen Rivaldo Junior Robin Hellemans Rody Davis Rom Rom Brillout Roman Roman Imankulov Romuald Brillout Rongjian Zhang Ruicong Ryan Christian Ryan Moyo Ryan Tsao Ryan Wang Rémi Emonet Sacha STAFYNIAK Sachin Raja Sam Sam Richard Sam Verschueren Samuel Alev Saurabh Daware Sean Deng Sean Watters Sebastian Krüger Sebastian Seilund SegaraRai Segev Finer Sepehr Safari Sepush Sergey Vinogradov SeyyedKhandon Shahriar Shojib Sharvilak Shigma Shinigami Shinigami92 Shyam Chen Shyim Simon Simon H Simon Legner Simon · 西萌 Sirui Chen Sleepy Five Sociosarbis Songkeys Sosuke Suzuki Stefan van Herwijnen Steve Lee Steve Shreeve Steven T Steven Waters Sting Alleman Sunil Pai Surmon Sven Svilen Ivanov Swedish-li TJ Koblentz TMQ Takashi MIMA Tal500 Tan Li Hau Tedy Thai Pangsakulyanont Theo Browne Thorsten Lünborg Tiep Nguyen Tiger Oakes Tim MacDonald Timo Zander Timsonrobl Titouan Mathis Tmk Tobias Lundin Tobias Melén Tobias Nießen Tomas Beres Tomasz Surowiec Tommaso De Rossi Tomáš Livora Tony Schaffert Tony Trinh Touchumind TrickyPi TropicalRaisel Troy Latchman Turmiht Tyler Rockwood Umidbek Karimov Valentin Vben Victor Victor Eke Victor Trigo Wagner Vitaly Baev Vladimir Vojtech Miksu WORMSS Wang Jian Wang Yifei Wei Wenlu Wang Wes Harper William Xavier Cambar Xin Du (Clark) Xuguang Mei Yadhunandan S Yan Savinov Yanlin Jiang Yaroslav Dobzhanskij Yash Singh Yehyoung Kang Yelmor Yiyang Sun Yoshi Togami Yunfei He Yunwei Xiao ZHAO Jinxiang ZYSzys Zahin Afsar Zehua Chen Zetao Zhuang Zhang Zhi Zhao Zheeeng ZhengX Zoran Pesic ZuoChenxue ZxBing0066 abderrahmen lahmedi agoni1212 alex alykamb amandeep-r andrew0 andyjxli ashish bbbboom bbotto-pdga bompus btea bz-pepite casret chaxus chencheng (云谦) chenjiajian chlorine chris-zhu circle-gon cisen clocher Zhong codeoD codpoe csr632 cyitao davidbielik dayuan dengqing dependabot[bot] dragonHandsome edison enya falstack ferdinando-ferreira fi3ework finico fishDog ghostdevv gnehc haiya6 hardfist heart hehe hemengke hiro hiro-lapis hiroki hisland hongmin.qiao hyrious idler ikeq imShara itibbers j000 jbmolle jods jsyanyang kakachake kamilic kazuya kawaguchi krystal kuake leemove li.li libing lijialiang lijianzhang likui liuwei lsdsjy ltaoo luke miles meijin meteorlxy metonym milahu mj2ks mori yuta moyinzi mstoller-centricity musicq mwyywm neal nio89 ocavue odanado okxiaoliang4 oliverpool patak patak-dev patak-js pengbo plq pooya parsa poyoho psaren qicoo qmhc rainkolwa reid j sherman renovate[bot] rhan rixo roger6106 rs3d rxliuli sagargurtu sanyuan sapphi-red shengxinjing shir0u shiyu shj sibbng simohamed smeng9 smnhgn spencer17x ssh stefnotch stygian-desolator sun0day swandir taisei mima tessaSAC tianyu94 toSayNothing toshify tuchg underfin untp vaakian X valley vemoo virgosoy vwkd wang wangkaiwd webfansplz wmzy wxy xiejiahe xingxiuyi xrkffgg xyl66 yArna yangxingyuan ygj6 yinzhenyu yoho ysy945 yuangongji yue yuuang yuz zdw zhangenming zhangrenyang zhenzhenChange ziga zk4 zoomdong zygzagZ zz İsmail Arılık ᴜɴвʏтᴇ ⚡ Aditya Patel ⚡ 三咲智子 三毛 云游君 似水微寒 兰天游 刘巍峰 宇cccc 宋来瑞 宋铄运 (Alan Song) 弘树@阿里 思翊 文蔺 李墨 杨兆雨 杨永安 沈青川 沧浪 清尘 王凯 粒粒橙 翠 / green 耗子 胡金鑫 花开半亩地 草鞋没号 蜗牛老湿 遊真uZ 那里好脏不可以 马鹏达 Ben Tea ```
matthewmayer commented 1 year ago

instead of [\w_\. ']+ you could just do something like [^<]+ as < will be the first char of the email address.

i noticed if i do git config user.name "matt < hello" then my author line looks like Author: matt hello <matt@example.com>

so it seems to be stripping out angle brackets to avoid confusion with the email

matthewmayer commented 1 year ago

bit more info on stripping:

https://github.com/libgit2/libgit2/issues/5342

Shinigami92 commented 1 year ago

Now we coming closer to an appropriated solution 👍

xDivisionByZerox commented 1 year ago

So we should actually refactor the implementation to normalize the username/full name before using it in the commit entry.

ST-DDT commented 1 year ago

What are you referring to with normalization? IMO we should make sure we pass the correct validation, but for that we need to actually have the correct validation.

Shinigami92 commented 1 year ago

What are you referring to with normalization? IMO we should make sure we pass the correct validation, but for that we need to actually have the correct validation.

They mean normalize the generated values inside commitEntry to strip out invalid chars

https://github.com/faker-js/faker/blob/1ae2f6f489bcf7b317202877af3419ecc01bc1d0/src/modules/git/index.ts#L108-L122

Like doing an extra step in line 114 for variable user

matthewmayer commented 1 year ago

Hmm how would you test that though considering fullName never includes a <