Skip to content

Add Property Tests and fix found issues#20

Merged
maennchen merged 1 commit intomainfrom
stream_data
Mar 4, 2025
Merged

Add Property Tests and fix found issues#20
maennchen merged 1 commit intomainfrom
stream_data

Conversation

@maennchen
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Mar 4, 2025

Pull Request Test Coverage Report for Build 8928600899abad2afdd608ef2897e01ee3f16039-PR-20

Details

  • 50 of 56 (89.29%) changed or added relevant lines in 4 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+1.5%) to 86.316%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/purl/generator.ex 23 25 92.0%
lib/purl/parser.ex 3 7 42.86%
Files with Coverage Reduction New Missed Lines %
lib/purl/parser.ex 1 81.82%
Totals Coverage Status
Change from base Build 24a3dff242df079efe501bbdc0177fac5ebb342f: 1.5%
Covered Lines: 164
Relevant Lines: 190

💛 - Coveralls

@maennchen
Copy link
Member Author

@matt-phylum Do you have time for a review of this? You seem to know your way around purl quite well :)

I saw that there's some ongoing discussions in the Purl Spec repo around escaping. That probably should be cleaned up a bit. (For example: are : escaped in the version?)

Right now the code is passing different characters around for each field.

The current state passes the tests and I can test the property purl |> compose |> parse == purl.

@maennchen maennchen self-assigned this Mar 4, 2025
@maennchen maennchen marked this pull request as ready for review March 4, 2025 19:16
Copy link
Contributor

@matt-phylum matt-phylum left a comment

Choose a reason for hiding this comment

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

LGTM I'm not an Elixir expert and I didn't verify that the sets of characters being encoded are exactly right according to the spec, but they're probably right if you're passing tests.

@maennchen maennchen merged commit c4549b3 into main Mar 4, 2025
9 checks passed
@maennchen maennchen deleted the stream_data branch March 4, 2025 21:16
@maennchen
Copy link
Member Author

Thanks for the review @matt-phylum :)

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