🚇🩹 Fix test CI workflow by s-weigand · Pull Request #129 · viu-media/viu

s-weigand · 2025-08-17T12:06:08Z

This partially fixes the broken test CI workflow, up to the type checking.

For the missing lxml types there is a stub package types-lxml.
But honestly, I would just use beautifulsoup4 (which also supports lxml as parser) to work with HTML and call it a day 😅

Hope this will help, cheers 🍺

This reverts commit c6b8cfc.

Copilot

Pull Request Overview

This PR fixes broken test CI workflow by addressing linting issues and dependency problems. The main focus is on resolving E722 linting errors (bare except clauses) and adding necessary system dependencies for the CI environment.

Adds # noqa: E722 comments to suppress linting warnings for bare except clauses
Removes unused TYPE_CHECKING import that was causing issues
Adds system dependencies for dbus-python in the GitHub Actions workflow

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`viu_cli/libs/selectors/rofi/selector.py`	Suppresses E722 linting errors for bare except clauses in notification handling
`viu_cli/cli/service/feedback/service.py`	Suppresses E722 linting errors for bare except clauses across multiple notification methods
`viu_cli/cli/service/download/service.py`	Suppresses E722 linting errors for bare except clauses in download notification handling
`viu_cli/cli/commands/anilist/commands/downloads.py`	Removes unused `TYPE_CHECKING` import
`.github/workflows/test.yml`	Adds libdbus-1-dev and libglib2.0-dev system dependencies for CI

Copilot · 2025-08-17T12:07:17Z

                    timeout=2 * 60,
                )
-            except:
+            except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except Exception:

Copilot · 2025-08-17T12:07:18Z

                timeout=2 * 60,
            )
-        except:
+        except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Copilot · 2025-08-17T12:07:18Z

                return
-            except:
+            except:  # noqa: E722
                logger.warning("Using rofi without plyer for notifications")


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

logger.warning("Using rofi without plyer for notifications")

except ImportError:

logger.warning("plyer not installed; using rofi without desktop notifications")

except Exception as e:

logger.warning(f"Failed to send desktop notification: {e}")

Copilot · 2025-08-17T12:07:18Z

                )
                return
-            except:
+            except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except (ImportError, Exception):

Copilot · 2025-08-17T12:07:19Z

                )
                return
-            except:
+            except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except Exception:

Copilot · 2025-08-17T12:07:19Z

                )
                return
-            except:
+            except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except Exception:

Copilot · 2025-08-17T12:07:19Z

                )
                return
-            except:
+            except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except (ImportError, Exception):

Copilot · 2025-08-17T12:07:20Z

                        * 60,
                    )
-                except:
+                except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except Exception: # noqa: E722

Copilot · 2025-08-17T12:07:20Z

                    timeout=self.app_config.general.desktop_notification_duration * 60,
                )
-            except:
+            except:  # noqa: E722


Using bare except clauses is a poor practice as it catches all exceptions including system exit and keyboard interrupts. Consider catching specific exceptions like ImportError or Exception instead.

Suggested change

except: # noqa: E722

except Exception:

Benexl · 2025-08-17T13:51:41Z

they still aint passing lol.
and by the way what did you change in the lock file

Benexl · 2025-08-17T13:53:34Z

bs4 is not being used because of the unnecserray overhead of creating structured data. we are just looking for specific targets the stream urls.
i planned to fix the test issues later as i come up with a testing strategy

s-weigand · 2025-08-17T14:18:52Z

they still aint passing lol.
As mentioned, it is a partial 😅
Instead of failing at uv sync (see also #124 ) it now gets to the type checking fails there 😅

and by the way what did you change in the lock file
I just ran uv lock, which replaced

[[package]]
name = "viu"
version = "3.1.0"
source = { editable = "." }

with

[[package]]
name = "viu-cli"
version = "3.2.6"
source = { editable = "." }

Before that uv sync --frozen failed because the package name in pyproject.toml and uv.lock were different.
That also triggered adding the new dependencies and their whole tree (I see a wall of pyobjc 🙈)

bs4 is not being used because of the unnecserray overhead of creating structured data. we are just looking for specific targets the stream urls.

Ok fair enough didn't know that there are so many HTML responses to parse that bs4 would cause a measurable performance impact, just mentioned it because back in the day I did regex HTML parsing because I didn't know bs4 and shotgun shot myself in the foot 😅

Benexl · 2025-08-17T17:25:48Z

ohh alright 😂 .

s-weigand added 7 commits August 17, 2025 12:48

🚇🩹 Install dbus-python build dependencies

6895426

❌ Run on feature-branch

c6b8cfc

🚇🩹 Install libglib2.0-dev

66b2473

🩹 Relock uv.lock file due to changed package name

7b43145

🧹 Remove unused TYPE_CHECKING import

f6fedf0

🧹 Ignore blank except ruff rule

4d50cff

Revert "❌ Run on feature-branch"

4b310e6

This reverts commit c6b8cfc.

Copilot AI review requested due to automatic review settings August 17, 2025 12:06

Copilot AI reviewed Aug 17, 2025

View reviewed changes

Benexl merged commit b689760 into viu-media:master Aug 17, 2025
0 of 2 checks passed

s-weigand deleted the fix-ci branch August 17, 2025 17:43

-                logger.warning("Using rofi without plyer for notifications")
+            except ImportError:
+                logger.warning("plyer not installed; using rofi without desktop notifications")
+            except Exception as e:
+                logger.warning(f"Failed to send desktop notification: {e}")

Conversation

s-weigand commented Aug 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

Uh oh!

Benexl commented Aug 17, 2025

Uh oh!

Benexl commented Aug 17, 2025

Uh oh!

s-weigand commented Aug 17, 2025

Uh oh!

Benexl commented Aug 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants