build: validate `build-system` table schema #365

layday · 2021-09-29T17:15:27Z

Closes #364.

gaborbernat

src/build/__init__.py

tests/test_projectbuilder.py

henryiii

Once existing comments are addressed, LGTM.

FFY00 · 2021-09-29T19:56:23Z

tests/test_projectbuilder.py

-        match='`requires` must be an array of strings',
-    ):
-        build._parse_build_system_table(
+            pytest.raises(build.BuildSystemTableValidationError, match='`requires` is a required property'),


We can just parameterize the match.

https://github.com/FFY00/python-pep621/blob/f374fa2fe8f53fc3b24ef9e08b00870fef8fce30/tests/test_standard_metadata.py#L479

Not all of them raise.

Then split the test into one for the errors and one for the case it should pass.

Do you not like the way it's structured now or why do you want me to do that?

It's to remove duplication and complexity. If you split the test into two you can easily parameterize only the error string.

I hope 1dd2d35 is acceptable.

I still think it would be better to split the test into two, and validate the return of _parse_build_system_table, but I am not gonna block the PR because of that.

Done in 827f567.

FFY00 · 2021-09-29T19:57:47Z

tests/test_projectbuilder.py

+)
+def test_parse_build_system_table_type_validation(pyproject_toml, raises):
+    with raises:
+        build._parse_build_system_table(pyproject_toml)


I would rather instantiate a ProjectBuilder here.

I'd have to serialise them all as TOML, create a tmp dir for each and write them on disk - I don't think I can disentangle the disk I/O with the mocker.

I think you can, as you should be able to patch builtins.open, but I'll leave that up to you. This is just a nitpick.

I think I might leave that for another day then 😛

Closes pypa#364.

So that we don't have to keep adjusting these numbers.

layday requested review from FFY00 and gaborbernat as code owners September 29, 2021 17:15

layday force-pushed the add-build-system-validation branch 2 times, most recently from c6c66f2 to f2ecee6 Compare September 29, 2021 17:17

gaborbernat approved these changes Sep 29, 2021

View reviewed changes

FFY00 reviewed Sep 29, 2021

View reviewed changes

src/build/__init__.py Outdated Show resolved Hide resolved

tests/test_projectbuilder.py Outdated Show resolved Hide resolved

henryiii approved these changes Sep 29, 2021

View reviewed changes

FFY00 reviewed Sep 29, 2021

View reviewed changes

FFY00 approved these changes Sep 29, 2021

View reviewed changes

layday force-pushed the add-build-system-validation branch from 1dd2d35 to 9c6356a Compare September 29, 2021 21:11

FFY00 enabled auto-merge (rebase) September 29, 2021 21:14

layday added 2 commits September 30, 2021 00:21

build: validate build-system table schema

827f567

Closes pypa#364.

tests: only check lineno of message logged in test module

ab4526d

So that we don't have to keep adjusting these numbers.

layday force-pushed the add-build-system-validation branch from 9c6356a to ab4526d Compare September 29, 2021 21:21

FFY00 merged commit 57e6cf3 into pypa:main Sep 29, 2021

layday deleted the add-build-system-validation branch March 1, 2024 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: validate `build-system` table schema #365

build: validate `build-system` table schema #365

layday commented Sep 29, 2021

gaborbernat left a comment

henryiii left a comment

FFY00 Sep 29, 2021

layday Sep 29, 2021

FFY00 Sep 29, 2021

layday Sep 29, 2021

FFY00 Sep 29, 2021

layday Sep 29, 2021

FFY00 Sep 29, 2021

layday Sep 29, 2021 •

edited

FFY00 Sep 29, 2021

layday Sep 29, 2021

FFY00 Sep 29, 2021

layday Sep 29, 2021

build: validate build-system table schema #365

build: validate build-system table schema #365

Conversation

layday commented Sep 29, 2021

gaborbernat left a comment

Choose a reason for hiding this comment

henryiii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

layday Sep 29, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

build: validate `build-system` table schema #365

build: validate `build-system` table schema #365

layday Sep 29, 2021 •

edited