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

Clean test files #7636

Closed
wants to merge 77 commits into from
Closed

Conversation

liviuconcioiu
Copy link
Collaborator

Fixes #7633

@liviuconcioiu
Copy link
Collaborator Author

@sanchezzzhak I've made some changes to rewriteTestFixtureFiles.php to remove the whitespace from #6938 and ran it for all files. clienthints.yml isn't completly cleaned and a few tests from other files because the script messes it up. See #7637.

@sanchezzzhak
Copy link
Collaborator

Only the versions that look like a int/float needed to be wrapped in a string
Example

version: 10
version: 1.0
version: 2.6

must be converted to

version: "10"
version: "1.0"
version: "2.6"

for version: 10.50.30, you don't need to do this
also, if square brackets occur in a string, this string must be wrapped in quotation marks.

in fact, when we use when creating a fixture through a tool mist/test.php we always get the correct test fixture

php misc/test.php "Mozilla/5.0 (Linux; arm_64; Android 13; CPH2333) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.6099.2552 YaApp_Android/24.10.1 YaSearchBrowser/24.10.1 BroPP/1.0 SA/3 Mobile Safari/537.36"

result

user_agent: Mozilla/5.0 (Linux; arm_64; Android 13; CPH2333) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.6099.2552 YaApp_Android/24.10.1 YaSearchBrowser/24.10.1 BroPP/1.0 SA/3 Mobile Safari/537.36
os:
  name: Android
  version: "13"
  platform: ARM
client:
  type: browser
  name: Yandex Browser
  version: 24.10.1
  engine: Blink
  engine_version: 120.0.6099.2552
device:
  type: smartphone
  brand: OPPO
  model: A94
os_family: Android
browser_family: Unknown

Unfortunately, the PHP yaml parser is not very strict, unlike python or nodejs.

I suggest not adding this PR, as we have resolved the main criticisms. #7634

@liviuconcioiu liviuconcioiu mentioned this pull request Apr 9, 2024
@liviuconcioiu liviuconcioiu deleted the rewrite branch April 20, 2024 02:39
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 this pull request may close these issues.

Wrong record test fixture for clents/library.yml
2 participants