#5385 Fix query filter for date ranges
Merged 8 months ago by ngompa. Opened 11 months ago by zlopez.
zlopez/pagure fix_query  into  master

file modified
+2 -2
@@ -4656,12 +4656,12 @@ 

          query = query.filter(model.PullRequest.date_created <= created_until)

  

      if updated_since:

-         query = query.filter(model.PullRequest.updated_on <= updated_since)

+         query = query.filter(model.PullRequest.updated_on >= updated_since)

      if updated_until:

          query = query.filter(model.PullRequest.updated_on <= updated_until)

  

      if closed_since:

-         query = query.filter(model.PullRequest.closed_at <= closed_since)

+         query = query.filter(model.PullRequest.closed_at >= closed_since)

      if closed_until:

          query = query.filter(model.PullRequest.closed_at <= closed_until)

  

@@ -1214,7 +1214,7 @@ 

          )

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data["requests"]), 0)

+         self.assertEqual(len(data["requests"]), 6)

  

          yesterday = today - datetime.timedelta(days=1)

          output = self.app.get(
@@ -1223,7 +1223,7 @@ 

          )

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data["requests"]), 0)

+         self.assertEqual(len(data["requests"]), 6)

  

          tomorrow = today + datetime.timedelta(days=1)

          output = self.app.get(
@@ -1232,7 +1232,7 @@ 

          )

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data["requests"]), 6)

+         self.assertEqual(len(data["requests"]), 0)

  

      @patch("pagure.lib.notify.send_email")

      def test_api_view_user_requests_filed_closed(self, mockemail):
@@ -1598,7 +1598,7 @@ 

          )

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data["requests"]), 0)

+         self.assertEqual(len(data["requests"]), 6)

  

          yesterday = today - datetime.timedelta(days=1)

          output = self.app.get(
@@ -1607,7 +1607,7 @@ 

          )

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data["requests"]), 0)

+         self.assertEqual(len(data["requests"]), 6)

  

          tomorrow = today + datetime.timedelta(days=1)

          output = self.app.get(
@@ -1616,7 +1616,7 @@ 

          )

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data["requests"]), 6)

+         self.assertEqual(len(data["requests"]), 0)

  

      @patch("pagure.lib.notify.send_email")

      def test_api_view_user_requests_actionable_closed(self, mockemail):

The query for pull request of user incorrectly worked with date ranges and
instead it assumed that since is lesser or equal than the date we are comparing
to.

This commit is fixing that behavior.

Signed-off-by: Michal Konečný mkonecny@redhat.com

that feels like something that would affect the unit-tests

@pingou You are right let me fix the tests

1 new commit added

  • Fix tests
11 months ago

rebased onto eda4e6f7610e6abf5347b1929499f63b5681958f

10 months ago

@zlopez could you squash the commit for fixing the tests into the main commit?

Actually, it looks like the tests fail now. :frowning:

Test failure summary:

 =========================== short test summary info ============================
16:07:42  FAILED tests/test_pagure_flask_ui_issues_open_access.py::PagureFlaskIssuesOpenAccesstests::test_view_issue
16:07:42  FAILED tests/test_pagure_flask_ui_issues_open_access.py::PagureFlaskIssuesOpenAccesstests::test_view_issue_user_ticket
16:07:42  FAILED tests/test_pagure_flask_ui_login.py::PagureFlaskLogintests::test_settings_admin_session_timedout
16:07:42  FAILED tests/test_pagure_flask_ui_issues.py::PagureFlaskIssuestests::test_view_issue
16:07:42  FAILED tests/test_pagure_flask_ui_issues.py::PagureFlaskIssuestests::test_view_issue_author
16:07:42  FAILED tests/test_pagure_flask_ui_issues.py::PagureFlaskIssuestests::test_view_issue_inconsistent_milestone
16:07:42  FAILED tests/test_pagure_flask_ui_issues.py::PagureFlaskIssuestests::test_view_issue_user_ticket
16:07:42  FAILED tests/test_pagure_flask_ui_issues_acl_checks.py::PagureFlaskIssuesACLtests::test_view_issue_admin_access
16:07:42  FAILED tests/test_pagure_flask_ui_issues_acl_checks.py::PagureFlaskIssuesACLtests::test_view_issue_no_access
16:07:42  FAILED tests/test_pagure_flask_ui_issues_acl_checks.py::PagureFlaskIssuesACLtests::test_view_issue_ticket_access
16:07:42  ==== 10 failed, 1694 passed, 8 skipped, 8684 warnings in 967.24s (0:16:07) =====
16:07:42  ERROR: InvocationError for command /pagure/.tox/py38/bin/pytest -n auto tests (exited with code 1)

Let me check that again

rebased onto 924fad07f2d1811033ac86ee6d73cb1c1c0df0a3

10 months ago

I fixed the tests and squashed the commits, hopefully the fixes didn't introduced another failures.

This fix fixed it for the pip environment, but introduced failures to f36 container. Should we update the container to F38?

I tried to update Fedora container to F36 and got 329 failed tests locally. I can try to fix them, but I'm not sure if that is worth the effort.

Let me try it with F37 first.

I tried to update Fedora container to F36 and got 329 failed tests locally. I can try to fix them, but I'm not sure if that is worth the effort.

Let me try it with F37 first.

We using F36 since https://pagure.io/pagure/pull-request/5383 and the unit tests are healthy right now. So I have the impression, if you see hundreds of failed tests locally, it might then just be related to your changes?

Also if you test with F37 and the pipeline is still running rpm and pip against f36, you might see again different results, so I suggest you test locally with the same container as we do in CI?

Maybe the changes introduced in https://pagure.io/pagure/pull-request/5384 help that your local testing is less time consuming so you can iterate more often to fix the problems.

But F36 is not supported anymore and the tests are failing for PyPI packages, because there was some change that isn't in RPMS packaged in F36.

I'm just trying if it's possible to update to F37/F38. I will create another PR for this change anyway.

But F36 is not supported anymore

Indeed, since 14 days, time is running fast...

You try F37 without your changes based on the latest master branch right?

Yes, I'm trying the changes currently on master branch.

Both F37 and F38 are running with python 3.11, which is probably causing so many tests to fail.

rebased onto c51d164f0c25612a6f8339af583b2cb40e88cd0b

10 months ago

rebased onto c51d164f0c25612a6f8339af583b2cb40e88cd0b

10 months ago

rebased onto 93b47635c1ca98b572cb2b5cc49949b23d454ac1

10 months ago

The tests should be fixed and the commits are squashed.

I wonder about the changed behavior of this, probably because of an updated python package or something but not part of the actual PR. So maybe it would be better to move this login url assert fixes into a separate PR, that we then can merge first, to not lose track?

@zlopez Could you separate the unrelated test fixes into a separate commit? Then I can merge this and we'd be able to track this change separately in the tree.

I'm working on moving the test containers to F38, so I will just remove the fixes from this PR and just put them in the work I'm doing right now.

rebased onto 7ade197069ddb725f05bfda9966d7493373eae14

9 months ago

Done, there are only the files related to this change in the PR now.

rebased onto 4b37990

8 months ago

pretty please pagure-ci rebuild

8 months ago

Only one test fix needed and then I can merge this:

15:52:11  =================================== FAILURES ===================================
15:52:11  _______________________ TestStyle.test_code_with_flake8 ________________________
15:52:11  [gw5] linux -- Python 3.10.11 /usr/bin/python3
15:52:11  
15:52:11  self = <tests.test_style.TestStyle testMethod=test_code_with_flake8>
15:52:11  
15:52:11      def test_code_with_flake8(self):
15:52:11          """Enforce PEP-8 compliance on the codebase.
15:52:11      
15:52:11          This test runs flake8 on the code, and will fail if it returns a
15:52:11          non-zero exit code.
15:52:11          If flake8 is not installed, this test auto-skips.
15:52:11          """
15:52:11          try:
15:52:11              import flake8
15:52:11          except ImportError as e:
15:52:11              raise unittest.SkipTest(
15:52:11                  "flake8 is not installed, skipping flake8 style check..."
15:52:11              )
15:52:11          # We ignore E712, which disallows non-identity comparisons with True and False
15:52:11          # We ignore W503, which disallows line break before binary operator
15:52:11          flake8_command = [
15:52:11              sys.executable,
15:52:11              "-m",
15:52:11              "flake8",
15:52:11              "--ignore=E712,W503,E203,E902,I201,I100",
15:52:11              "--max-line-length=80",
15:52:11              REPO_PATH,
15:52:11          ]
15:52:11      
15:52:11          # check if we have an old flake8 or not
15:52:11          import flake8
15:52:11      
15:52:11          flake8_v = flake8.__version__.split(".")
15:52:11          for idx, val in enumerate(flake8_v):
15:52:11              try:
15:52:11                  val = int(val)
15:52:11              except ValueError:
15:52:11                  pass
15:52:11              flake8_v[idx] = val
15:52:11          old_flake = tuple(flake8_v) < (3, 0)
15:52:11      
15:52:11          if old_flake:
15:52:11              raise unittest.SkipTest("Flake8 version too old to be useful")
15:52:11      
15:52:11          proc = subprocess.Popen(
15:52:11              flake8_command, stdout=subprocess.PIPE, cwd=REPO_PATH
15:52:11          )
15:52:11          print(proc.communicate())
15:52:11      
15:52:11  >       self.assertEqual(proc.returncode, 0)
15:52:11  E       AssertionError: 1 != 0
15:52:11  
15:52:11  tests/test_style.py:76: AssertionError
15:52:11  ----------------------------- Captured stdout call -----------------------------
15:52:11  (b'/pagure/pagure/cli/admin.py:1071:81: E501 line too long (81 > 80 characters)\n', None)
15:52:11  =============================== warnings summary ===============================

Pull-Request has been merged by ngompa

8 months ago

Actually the failure is not your fault, so I'll fix it separately. Merged, thanks!