-
Notifications
You must be signed in to change notification settings - Fork 917
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
PE parser: add unit tests and remove usage of arrow module #6481
PE parser: add unit tests and remove usage of arrow module #6481
Conversation
…er to have full data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Just one tiny comment to take a look at then I'll merge this.
"production": element["production"], | ||
"source": element["source"], | ||
"zoneKey": element["zoneKey"], | ||
"sourceType": "measured", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"sourceType": "measured", | |
"sourceType": element["sourceType"].value, |
This just brings it in line with our other tests and makes it easy to update if we ever change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing we can do if we want to reduce the size of these are minifying the files, it should have no impact on the tests.
Merging this as is right now as we have a follow up PR that converts it to use parser event lists as well. |
👍 |
Issue
#6135
Description
This PR adds unit tests for the PE parser and removes usage of the
arrow
module from the PE parser.Preview
Double check
poetry run test_parser "zone_key"
pnpx prettier@2 --write .
andpoetry run format
in the top level directory to format my changes.