npm should not be required to install pytest-html from pypi #747
Comments
thebaptiste
changed the title
|
hmm... that's weird. Like you say, it shouldn't be required. It's only required when building from source. It's somehow related to Hatch and/or pyproject.toml - not sure how or why tho. |
|
pytest-html installs just fine on a system without npm. jbrwe6@MAC-6CY79W0M dev % mkdir testing-npm
jbrwe6@MAC-6CY79W0M dev % cd testing-npm
jbrwe6@MAC-6CY79W0M testing-npm % python3 -m venv .venv
jbrwe6@MAC-6CY79W0M testing-npm % source .venv/bin/activate
(.venv) jbrwe6@MAC-6CY79W0M testing-npm % pip freeze
(.venv) jbrwe6@MAC-6CY79W0M testing-npm % npm
zsh: command not found: npm
(.venv) jbrwe6@MAC-6CY79W0M testing-npm % pip install pytest-html
Collecting pytest-html
Downloading pytest_html-4.0.2-py3-none-any.whl (23 kB)
*snip*
Installing collected packages: tomli, pluggy, packaging, iniconfig, exceptiongroup, pytest, MarkupSafe, pytest-metadata, jinja2, pytest-html
Successfully installed MarkupSafe-2.1.3 exceptiongroup-1.1.3 iniconfig-2.0.0 jinja2-3.1.2 packaging-23.2 pluggy-1.3.0 pytest-7.4.3 pytest-html-4.0.2 pytest-metadata-3.0.0 tomli-2.0.1
(.venv) jbrwe6@MAC-6CY79W0M testing-npm % pip freeze
exceptiongroup==1.1.3
iniconfig==2.0.0
Jinja2==3.1.2
MarkupSafe==2.1.3
packaging==23.2
pluggy==1.3.0
pytest==7.4.3
pytest-html==4.0.2
pytest-metadata==3.0.0
tomli==2.0.1So unless you can provide steps to reproduce, I'm closing the ticket. |
|
It seems like the sdist needs it still, I'd recommend checking if the build results can be included and used for the sdist as well to ease things for people that work with broken ci setups or repackage |
Aha, so something (likely in the pyproject.toml) is missing specifically for the sdist? Into the Hatch documentation I go then... |
|
Hmm... when I build with Hatch, the However, it seems that the zipped and tarred assets in the Github releases does not. Something I need to test, is what the result of the way CI (using |
|
Building with I'm out of ideas. Without steps to reproduce, I can't fix this. |
|
|
so the build hook is always used @ofek is there builtin hatch helper to have certain build steps happen for from source but not for from sdist |
|
If the response from Ofek is negative, what's the solution? Remove the build-hook and build it "manually" in CI and make sure |
|
The fix should be pretty simple https://github.com/pytest-dev/pytest-html/blob/master/scripts/npm.py Just gate those two statements by a check for something that is not shipped in either artifact like |
You mean like: import subprocess
from hatchling.builders.hooks.plugin.interface import BuildHookInterface
from pathlib import Path
class NpmBuildHook(BuildHookInterface):
def initialize(self, version, build_data):
app_js = Path("src/pytest_html/resources/app.js")
if not app_js.exists():
subprocess.check_output("npm ci", shell=True)
subprocess.check_output("npm run build", shell=True) |
|
Yes but just for correctness/to be pedantic even though it's the same path: app_js = Path(self.root, "src", "pytest_html", "resources", "app.js")Also I do understand what you are going for there but that would not work for repeat builds like running it twice locally which is why I recommended something that is not shipped at all if possible. |
Ah, I see what you mean now! If Can there be cases where someones building from source but import subprocess
from hatchling.builders.hooks.plugin.interface import BuildHookInterface
from pathlib import Path
class NpmBuildHook(BuildHookInterface):
def initialize(self, version, build_data):
building_from_source = Path(self.root, ".git").exists()
if building_from_source:
subprocess.check_output("npm ci", shell=True)
subprocess.check_output("npm run build", shell=True) |
|
Oh yeah definitely you're right, forget what I said and continue with the path that you chose. A situation would be an archive from a GitHub release. You build artifacts in CI the proper way and it would only run once so it's fine. |
|
Upon further reflection perhaps you can take care of that scenario: if that directory exists at the same time as the file existing then assume a local development setup and always trigger a build. |
Now I'm confused. 😅 |
|
My apologies! Your proposal is perfect I was just trying to find a way to have local development builds always trigger JavaScript builds and for that I was thinking if |
Ah. So I should stick with my initial solution? 😅 🙇 app_js = Path(self.root, "src", "pytest_html", "resources", "app.js")
if not app_js.exists(): |
|
Many thanks for the fix ! |
Since release 4.0.0rc5, npm is required to install pytest-html from pypi
I understand npm is only used for tests purpose (maybe I'm wrong...) ? If it is the case I think it should not be necessary to have npm pre-installed to install pytest-html
Below the error I have installing pytest-html >= 4.0.0rc5 from pypi (same thing with 4.0.0, 4.0.1 and 4.0.2) :
The text was updated successfully, but these errors were encountered: