Merge pull request #91 from GjjvdBurg/bugfix/have_node

Improve the check for Node

Makefile

@@ -53,7 +53,7 @@ dist: ## Make Python source distribution
 .PHONY: test
 
 test: venv ## Run unit tests
-	source $(VENV_DIR)/bin/activate && python -m pytest -v . --cov readabilipy --cov-report term-missing --benchmark-disable
+	source $(VENV_DIR)/bin/activate && cd $(TEST_DIR) && python -m pytest -v . --cov readabilipy --cov-report term-missing --benchmark-disable
 	source $(VENV_DIR)/bin/activate && pyflakes *.py readabilipy $(TEST_DIR)
 	source $(VENV_DIR)/bin/activate && pycodestyle --statistics --ignore=E501 --count *.py readabilipy $(TEST_DIR)
 	source $(VENV_DIR)/bin/activate && pylint readabilipy $(TEST_DIR)/*.py
@@ -79,7 +79,7 @@ docs: install ## Build documentation with Sphinx
 # Virtual environment #
 #######################
 
-.PHONY: venv
+.PHONY: venv clean_venv
 
 venv: $(VENV_DIR)/bin/activate
 
@@ -88,6 +88,9 @@ $(VENV_DIR)/bin/activate: setup.py
 	source $(VENV_DIR)/bin/activate && pip install .[dev]
 	touch $(VENV_DIR)/bin/activate
 
+clean_venv:
+	rm -rf $(VENV_DIR)
+
 ############
 # Clean up #
 ############

readabilipy/__main__.py

@@ -8,7 +8,7 @@
 import json
 
 from .__version__ import __version__
-from .simple_json import simple_json_from_html_string
+from .simple_json import simple_json_from_html_string, have_node
 
 
 def main():
@@ -50,7 +50,7 @@ def main():
         "--version",
         help="Show version and exit",
         action="version",
-        version=__version__,
+        version=f"{__version__} (Readability.js supported: {'yes' if have_node() else 'no'})",
     )
 
     args = parser.parse_args()

readabilipy/simple_json.py

@@ -17,12 +17,21 @@ def have_node():
     """Check that we can run node and have a new enough version """
     try:
         cp = subprocess.run(['node', '-v'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False)
-        returncode = cp.returncode
     except FileNotFoundError:
         return False
 
+    if not cp.returncode == 0:
+        return False
+
     major = int(cp.stdout.split(b'.')[0].lstrip(b'v'))
-    return returncode == 0 and major >= 10
+    if major < 10:
+        return False
+
+    # check that this package has a node_modules dir in the javascript
+    # directory, if it doesn't, it wasn't installed with Node support
+    jsdir = os.path.join(os.path.dirname(__file__), 'javascript')
+    node_modules = os.path.join(jsdir, 'node_modules')
+    return os.path.exists(node_modules)
 
 
 def simple_json_from_html_string(html, content_digests=False, node_indexes=False, use_readability=False):

readabilipy/simplifiers/html.py

@@ -83,7 +83,7 @@ def remove_metadata(soup):
     We were a bit worried about potentially removing content here but satisfied
     ourselves it won't be displayed by most browsers in most cases
     (see https://github.com/alan-turing-institute/ReadabiliPy/issues/32)"""
-    for comment in soup.findAll(string=lambda text: any([isinstance(text, x) for x in [Comment, Doctype]])):
+    for comment in soup.findAll(string=lambda text: any(isinstance(text, x) for x in [Comment, Doctype])):
         comment.extract()
 
 

tests/test_simple_json.py

@@ -1,8 +1,12 @@
+
+from subprocess import CompletedProcess
+from unittest import mock
+
 # from .checks import check_extract_article
 from bs4 import BeautifulSoup
 from readabilipy import simple_json_from_html_string
 from readabilipy.simplifiers import normalise_text
-from readabilipy.simple_json import plain_element, plain_text_leaf_node, add_node_indexes, content_digest
+from readabilipy.simple_json import plain_element, plain_text_leaf_node, add_node_indexes, content_digest, have_node
 
 
 def test_empty_page():
@@ -88,3 +92,32 @@ def test_content_digest_assignment():
     assert digests == ['5271913f47bd4cbfda56ff8c0cddfc481d6bc4fe99725906068fbb6144bfeab4',
                        '4c2e9e6da31a64c70623619c449a040968cdbea85945bf384fa30ed2d5d24fa3',
                        '']
+
+
+@mock.patch('subprocess.run')
+def test_have_node_1(mock_subprocess_run):
+    mock_subprocess_run.side_effect = FileNotFoundError("No such file or directory: 'node'")
+    assert not have_node()
+
+
+@mock.patch('subprocess.run')
+def test_have_node_2(mock_subprocess_run):
+    mock_subprocess_run.return_value = CompletedProcess("", 1)
+    assert not have_node()
+
+
+@mock.patch('subprocess.run')
+def test_have_node_3(mock_subprocess_run):
+    mock_subprocess_run.return_value = CompletedProcess("", 0, stdout=b"v9.0.0\n")
+    assert not have_node()
+
+
+@mock.patch('os.path.exists')
+def test_have_node_4(mock_os_path_exists):
+    mock_os_path_exists.return_value = False
+    assert not have_node()
+
+
+def test_have_node_5():
+    # Assumes we're running on a system with Node/Readability.js installed
+    assert have_node()