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

Update pdf tests #1732

Closed
wants to merge 15 commits into from
Closed

Update pdf tests #1732

wants to merge 15 commits into from

Conversation

emko-274
Copy link
Contributor

Updated tests to reflect new changes. Temporarily commenting out markdown rendering tests

Copy link
Contributor

@wittejm wittejm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far! Do you intend to add the markdown test back in for this PR?


county_fines = [x for x in record["summary"]["county_fines"]]

#fines_total = record["summary"]["total_fines_due"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still needed? Looks like it can be removed

@@ -13,11 +13,17 @@ def to_markdown(record: Dict, header: Optional[str] = None, aliases: List[Dict]
ineligible_case_charges = [
x for x in record["summary"]["charges_grouped_by_eligibility_and_case"] if x[0] == "Ineligible"
]

#print(ineligible_case_charges)
Copy link
Contributor

@wittejm wittejm Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's delete this to keep things tidier.

@emko-274
Copy link
Contributor Author

I'm stuck with the markdown tests -- how do I diff the expected and actual summary pdf files?

@wittejm
Copy link
Contributor

wittejm commented Jul 25, 2024

If you run docker-compose exec expungeservice pipenv run pytest ./tests/pdf/test_markdown_renderer.py -vv

it gives the full printout of the text diff for just the markdown test (-vv option is verbose). It looks like it's just a couple of spacing issues and other minor typos.

@emko-274
Copy link
Contributor Author

Okay, thank you. I've added the tests back and removed the comments.

@wittejm
Copy link
Contributor

wittejm commented Jul 26, 2024

Looks like the test is still commented out though?

@emko-274
Copy link
Contributor Author

Sorry, I think saved the changes to the wrong branch. I'll make a new pull request

@emko-274 emko-274 closed this Jul 26, 2024
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.

3 participants