From 9ff12c643feadc353f29fe8dac4b9819caf1779c Mon Sep 17 00:00:00 2001
From: Frederick Muriuki Muriithi
Date: Thu, 13 Jun 2024 15:54:17 -0500
Subject: Fix tests and issues caught by tests.

---
 qc_app/entry.py                             |  7 +--
 qc_app/templates/index.html                 |  8 +--
 tests/qc_app/test_entry.py                  | 97 +++++++++--------------------
 tests/qc_app/test_parse.py                  |  6 +-
 tests/qc_app/test_progress_indication.py    | 24 +++----
 tests/qc_app/test_results_page.py           |  6 +-
 tests/qc_app/test_uploads_with_zip_files.py | 49 +++++++++++----
 7 files changed, 91 insertions(+), 106 deletions(-)

diff --git a/qc_app/entry.py b/qc_app/entry.py
index d23a45a..f2db878 100644
--- a/qc_app/entry.py
+++ b/qc_app/entry.py
@@ -100,7 +100,7 @@ def upload_file():
     request_errors = errors(request)
     if request_errors:
         for error in request_errors:
-            flash(error, "alert-error error-expr-data")
+            flash(error, "alert-danger error-expr-data")
         return redirect(url_for("entry.upload_file"))
 
     filename = secure_filename(request.files["qc_text_file"].filename)
@@ -113,9 +113,8 @@ def upload_file():
     zip_errors = zip_file_errors(filepath, upload_dir)
     if zip_errors:
         for error in zip_errors:
-            flash(error, "alert-error error-expr-data")
-        return render_template(
-            "index.html", species=with_db_connection(species)), 400
+            flash(error, "alert-danger error-expr-data")
+        return redirect(url_for("entry.upload_file"))
 
     return redirect(url_for("parse.parse",
                             speciesid=request.form["speciesid"],
diff --git a/qc_app/templates/index.html b/qc_app/templates/index.html
index 997c1e5..e23dde1 100644
--- a/qc_app/templates/index.html
+++ b/qc_app/templates/index.html
@@ -26,7 +26,7 @@
     <p>The R/qtl2 bundle, additionally, can contain extra metadata, that neither
       of the methods below can handle.</p>
 
-    <a href={{url_for("upload.rqtl2.select_species")}}
+    <a href="{{url_for('upload.rqtl2.select_species')}}"
        title="Upload a zip bundle of R/qtl2 files">
       <button class="btn btn-primary">upload R/qtl2 bundle</button></a>
   </div>
@@ -60,9 +60,9 @@
     </ol>
   </div>
 
-  <a href={{url_for("entry.upload_file")}}
+  <a href="{{url_for('entry.upload_file')}}"
      title="Upload your expression data"
-     class="btn btn-primary">upload Samples/Cases</a>
+     class="btn btn-primary">upload expression data</a>
 </div>
 
 <div class="row">
@@ -75,7 +75,7 @@
     <p>This section gives you the opportunity to upload any missing samples</p>
   </div>
 
-  <a href={{url_for("samples.select_species")}}
+  <a href="{{url_for('samples.select_species')}}"
      title="Upload samples/cases/individuals for your data"
      class="btn btn-primary">upload Samples/Cases</a>
 </div>
diff --git a/tests/qc_app/test_entry.py b/tests/qc_app/test_entry.py
index bfe56ec..0c614a5 100644
--- a/tests/qc_app/test_entry.py
+++ b/tests/qc_app/test_entry.py
@@ -1,80 +1,43 @@
 """Test the entry module in the web-ui"""
 import pytest
 
-from tests.conftest import uploadable_file_object
+@pytest.mark.parametrize(
+    "dataitem,lower",
+    (
+        # expression data UI elements
+        (b'<h2 class="heading">expression data</h2>', True),
+        (b'<a href="/upload"', False),
+        (b'upload expression data</a>', False),
 
-def test_basic_elements_present_in_index_page(client):
-    """
-    GIVEN: A flask application testing client
-    WHEN: the index page is requested with the "POST" method and no datat
-    THEN: verify that the response contains error notifications
-    """
-    response = client.get("/")
-    assert response.status_code == 200
-    ## form present
-    assert b'<form action="/"' in response.data
-    assert b'method="POST"' in response.data
-    assert b'enctype="multipart/form-data"' in response.data
-    assert b'</form>' in response.data
-    ## filetype elements
-    assert b'<input type="radio" name="filetype"' in response.data
-    assert b'id="filetype_standard_error"' in response.data
-    assert b'id="filetype_average"' in response.data
-    ## file upload elements
-    assert b'<label for="file_upload"' in response.data
-    assert b'select file' in response.data
-    assert b'<input type="file" name="qc_text_file"' in response.data
-    assert b'id="file_upload"' in response.data
-    ## submit button
-    assert b'<input type="submit" value="upload file"' in response.data
+        # samples/cases data UI elements
+        (b'<h2 class="heading">samples/cases</h2>', True),
+        (b'<a href="/samples/upload/species"', False),
+        (b'upload samples/cases', True),
 
-def test_post_notifies_errors_if_no_data_is_provided(client):
+        # R/qtl2 data UI elements
+        (b'<h2 class="heading">r/qtl2 bundles</h2>', True),
+        (b'<a href="/upload/rqtl2/select-species"', False),
+        (b'upload r/qtl2 bundle', True)
+    ))
+def test_landing_page_has_sections(client, dataitem, lower):
     """
     GIVEN: A flask application testing client
-    WHEN: the index page is requested with the "POST" method and with no
-          data provided
-    THEN: ensure the system responds woit the appropriate error messages
+    WHEN: the index page is requested
+    THEN: ensure the page has the expected UI elements
     """
-    response = client.post("/", data={})
-    assert response.status_code == 400
-    assert b'Invalid file type provided.' in response.data
-    assert b'No file was uploaded.' in response.data
+    resp = client.get("/")
+    assert resp.status_code == 200
+    assert dataitem in (resp.data.lower() if lower else resp.data)
 
-def test_post_with_correct_data(client):
-    """
-    GIVEN: A flask application testing client
-    WHEN: the index page is requested with the "POST" method and with the
-          appropriate data provided
-    THEN: ensure the system redirects to the parse endpoint with the filename
-          and filetype
-    """
-    response = client.post(
-        "/", data={
-            "speciesid": 1,
-            "filetype": "average",
-            "qc_text_file": uploadable_file_object("no_data_errors.tsv")
-        })
-
-    assert response.status_code == 302
-    assert b'Redirecting...' in response.data
-    assert (
-        b'/parse/parse?speciesid=1&amp;filename=no_data_errors.tsv&amp;filetype=average'
-        in response.data)
 
-@pytest.mark.parametrize(
-    "request_data,error_message",
-    (({"filetype": "invalid_choice",
-       "qc_text_file": uploadable_file_object("no_data_errors.tsv")},
-      b'Invalid file type provided.'),
-     ({"filetype": "average"}, b'No file was uploaded.'),
-     ({"filetype": "standard-error"}, b'No file was uploaded.')))
-def test_post_with_missing_or_invalid_data(client, request_data,error_message):
+def test_landing_page_fails_with_post(client):
     """
     GIVEN: A flask application testing client
-    WHEN: the index page is requested with the "POST" method and with data
-          either being missing or invalid
-    THEN: ensure that the system responds with the appropriate error message
+    WHEN: the index page is requested with the "POST" method
+    THEN: ensure the system fails
     """
-    response = client.post("/", data=request_data)
-    assert response.status_code == 400
-    assert error_message in response.data
+    resp = client.post("/")
+    assert resp.status_code == 405
+    assert (
+        b'<h1>405: The method is not allowed for the requested URL.</h1>'
+        in resp.data)
diff --git a/tests/qc_app/test_parse.py b/tests/qc_app/test_parse.py
index 187af76..3915a4d 100644
--- a/tests/qc_app/test_parse.py
+++ b/tests/qc_app/test_parse.py
@@ -30,7 +30,7 @@ def test_parse_with_existing_uploaded_file(#pylint: disable=[too-many-arguments]
     filename = "no_data_errors.tsv"
     filetype = "average"
     client.post(
-        "/", data={
+        "/upload", data={
             "speciesid": speciesid,
             "filetype": filetype,
             "qc_text_file": uploadable_file_object(filename)})
@@ -81,7 +81,7 @@ def test_parse_with_non_uploaded_file(client, filename, uri, error_msgs):
     ## Conditionally upload files
     if filename and filename != "non_existent.file":
         client.post(
-        "/", data={
+        "/upload", data={
             "filetype": "average",
             "qc_text_file": uploadable_file_object(filename)})
     # Trigger
@@ -89,7 +89,7 @@ def test_parse_with_non_uploaded_file(client, filename, uri, error_msgs):
     ## Check that there was exactly one redirect
     assert len(resp.history) == 1 and resp.history[0].status_code == 302
     ## Check that redirect is to home page and is successful
-    assert resp.request.path == "/"
+    assert resp.request.path == "/upload"
     assert resp.status_code == 200
     ## Check that error(s) are displayed
     for error_msg in error_msgs:
diff --git a/tests/qc_app/test_progress_indication.py b/tests/qc_app/test_progress_indication.py
index 8eba970..14a1050 100644
--- a/tests/qc_app/test_progress_indication.py
+++ b/tests/qc_app/test_progress_indication.py
@@ -15,7 +15,7 @@ def test_with_non_existing_job(client, redis_conn_with_fresh_job): # pylint: dis
     assert (
         b"No job, with the id '<em>non-existent-job-id</em>' was found!"
         in resp.data)
-    assert b'<meta http-equiv="refresh" content="5;url=/">' in resp.data
+    assert b'<meta http-equiv="refresh" content="5;url=/upload">' in resp.data
 
 def test_with_unstarted_job(client, job_id, redis_conn_with_fresh_job): # pylint: disable=[unused-argument]
     """
@@ -29,9 +29,9 @@ def test_with_unstarted_job(client, job_id, redis_conn_with_fresh_job): # pylint
     resp = client.get(f"/parse/status/{job_id}")
     assert b'<meta http-equiv="refresh" content="5">' in resp.data
     assert (
-        b'<progress id="job_' +
-        (f'{job_id}').encode("utf8") +
-        b'" value="0.0">0.0</progress>') in resp.data
+        b'<progress id="job_' + (f'{job_id}').encode("utf8") + b'"') in resp.data
+    assert b'value="0.0"' in resp.data
+    assert b'0.0</progress>' in resp.data
 
 def test_with_in_progress_no_error_job(
         client, job_id, redis_conn_with_in_progress_job_no_errors): # pylint: disable=[unused-argument]
@@ -48,9 +48,9 @@ def test_with_in_progress_no_error_job(
     resp = client.get(f"/parse/status/{job_id}")
     assert b'<meta http-equiv="refresh" content="5">' in resp.data
     assert (
-        b'<progress id="job_' +
-        (f'{job_id}').encode("utf8") +
-        b'" value="0.32242342">32.242342</progress>') in resp.data
+        b'<progress id="job_' + (f'{job_id}').encode("utf8") + b'"') in resp.data
+    assert b'value="0.32242342"' in resp.data
+    assert b'32.242342</progress>' in resp.data
     assert (
         b'<span >No errors found so far</span>'
         in resp.data)
@@ -72,13 +72,13 @@ def test_with_in_progress_job_with_errors(
     resp = client.get(f"/parse/status/{job_id}")
     assert b'<meta http-equiv="refresh" content="5">' in resp.data
     assert (
-        b'<progress id="job_' +
-        (f'{job_id}').encode("utf8") +
-        b'" value="0.4534245">45.34245</progress>') in resp.data
+        b'<progress id="job_' + (f'{job_id}').encode("utf8") + b'"') in resp.data
+    assert b'value="0.4534245"' in resp.data
+    assert b'45.34245</progress>' in resp.data
     assert (
-        b'<p class="alert-error">We have found the following errors so far</p>'
+        b'<p class="alert-danger">We have found the following errors so far</p>'
         in resp.data)
-    assert b'table class="reports-table">' in resp.data
+    assert b'table class="table reports-table">' in resp.data
     assert b'Duplicate Header' in resp.data
     assert b'Invalid Value' in resp.data
 
diff --git a/tests/qc_app/test_results_page.py b/tests/qc_app/test_results_page.py
index 57f1531..bb596d7 100644
--- a/tests/qc_app/test_results_page.py
+++ b/tests/qc_app/test_results_page.py
@@ -56,12 +56,12 @@ def test_results_with_completed_job_some_errors(
           1. the system redirects to the results page
           2. the results page displays the errors found
     """
-    resp = client.get(f"/parse/status/{job_id}", follow_redirects=True)
+    resp = client.get(f"/parse/status/{job_id}", follow_redirects=True);import sys;print(resp.data, file=sys.stderr)
     assert len(resp.history) == 1
     assert (
-        b'<p class="alert-error">We found the following errors</p>'
+        b'<p class="alert-danger">We found the following errors</p>'
         in resp.data)
-    assert b'<table class="reports-table">' in resp.data
+    assert b'<table class="table reports-table">' in resp.data
     assert b'Duplicate Header' in resp.data
     assert b'<td>Heading &#39;DupHead&#39; is repeated</td>' in resp.data
     assert b'Invalid Value' in resp.data
diff --git a/tests/qc_app/test_uploads_with_zip_files.py b/tests/qc_app/test_uploads_with_zip_files.py
index fb101ad..1506cfa 100644
--- a/tests/qc_app/test_uploads_with_zip_files.py
+++ b/tests/qc_app/test_uploads_with_zip_files.py
@@ -8,10 +8,17 @@ def test_upload_zipfile_with_zero_files(client):
     THEN: Ensure that the system responds with the appropriate error message and
           status code
     """
-    resp = client.post("/", data={
-        "filetype": "average",
-        "qc_text_file": uploadable_file_object("empty.zip")})
-    assert resp.status_code == 400
+    resp = client.post("/upload",
+                       data={
+                           "filetype": "average",
+                           "qc_text_file": uploadable_file_object("empty.zip")},
+                       follow_redirects=True)
+    assert len(resp.history) == 1
+    redirect = resp.history[0]
+    assert redirect.status_code == 302
+    assert redirect.location == "/upload"
+
+    assert resp.status_code == 200
     assert (b"Expected exactly one (1) member file within the uploaded zip "
             b"file. Got 0 member files.") in resp.data
 
@@ -22,10 +29,18 @@ def test_upload_zipfile_with_multiple_files(client):
     THEN: Ensure that the system responds with the appropriate error message and
           status code
     """
-    resp = client.post("/", data={
-        "filetype": "average",
-        "qc_text_file": uploadable_file_object("multiple_files.zip")})
-    assert resp.status_code == 400
+    resp = client.post(
+        "/upload",
+        data={
+            "filetype": "average",
+            "qc_text_file": uploadable_file_object("multiple_files.zip")},
+        follow_redirects=True)
+    assert len(resp.history) == 1
+    redirect = resp.history[0]
+    assert redirect.status_code == 302
+    assert redirect.location == "/upload"
+
+    assert resp.status_code == 200
     assert (b"Expected exactly one (1) member file within the uploaded zip "
             b"file. Got 3 member files.") in resp.data
 
@@ -35,7 +50,7 @@ def test_upload_zipfile_with_one_tsv_file(client):
     WHEN: A zip file with exactly one valid TSV file is uploaded
     THEN: Ensure that the system redirects to the correct next URL
     """
-    resp = client.post("/", data={
+    resp = client.post("/upload", data={
         "speciesid": 1,
         "filetype": "average",
         "qc_text_file": uploadable_file_object("average.tsv.zip")})
@@ -53,9 +68,17 @@ def test_upload_zipfile_with_one_non_tsv_file(client):
     THEN: Ensure that the system responds with the appropriate error message and
           status code
     """
-    resp = client.post("/", data={
-        "filetype": "average",
-        "qc_text_file": uploadable_file_object("non_tsv.zip")})
-    assert resp.status_code == 400
+    resp = client.post(
+        "/upload",
+        data={
+            "filetype": "average",
+            "qc_text_file": uploadable_file_object("non_tsv.zip")},
+        follow_redirects=True)
+    assert len(resp.history) == 1
+    redirect = resp.history[0]
+    assert redirect.status_code == 302
+    assert redirect.location == "/upload"
+
+    assert resp.status_code == 200
     assert (b"Expected the member text file in the uploaded zip file to "
             b"be a tab-separated file.") in resp.data
-- 
cgit v1.2.3