Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pylint unit test test_lint.py should be executed "on demand" only (not as installation test on clients) #1634

Closed
aryoda opened this issue Feb 2, 2024 · 9 comments · Fixed by #1635

Comments

@aryoda
Copy link
Contributor

aryoda commented Feb 2, 2024

In BiT relese 1.4.3 the pylint unit test is included an is always executed to check the source code (static code analyzer).

This is not required for users and on distros who install BiT from the source code via make and execute the unit tests.

In case of ARCH linux it creates an unnecessary dependency and slows down the installation process:

https://aur.archlinux.org/pkgbase/backintime#comment-954673

I suggest to

  • either make the execution of this unit test optional (not by default)
  • or offer a way to disable it (a small documentation may suffice if this works)

A new Makefile target make dev-test similar to make test vs. make test-v could also be a valid solution
which is the only target that executes the linter (meant for us as developer)

@buhtz Do you have an idea?

@buhtz
Copy link
Member

buhtz commented Feb 3, 2024

Yeah, that is annoying of course. 😃
My workaround would be to skip the test when it is not TravisCI (via check on environment variable) or when pylint is not available on the current system.

Distinguishing between build and dev dependencies is easier when we have pyproject.toml (#1575). Also categorizing of unit tests will be easer then (#1489).

@buhtz
Copy link
Member

buhtz commented Feb 4, 2024

Just one thought. Don't know how Arch handles this. But do the unit tests run every time a user will install BIT or just when @graysky2 build the new package?

@graysky2
Copy link
Contributor

graysky2 commented Feb 4, 2024

It is only run when a user builds the package. Once, built, it is not required. Users can optionally skip the check function when building as well. I think it's fine as-is.

@buhtz
Copy link
Member

buhtz commented Feb 4, 2024

What is a "user" in Arch definition? Why should a "user" build a package? Users don't build they just download and install from my point of view.

@emtiu
Copy link
Member

emtiu commented Feb 4, 2024

(deleted a comment in which I confused Arch with Gentoo)

@graysky2
Copy link
Contributor

graysky2 commented Feb 4, 2024

Arch provides pre-compiled packages like Debian. Of course, not everything is packaged officially. Backintime is not officially supported so the best way for users of Arch to get the software is from an unofficial source. That is the Arch User Repository (AUR). It does not host compiled packages, it does host the user-maintained (me in this case) recipes to build and package software using the Arch native package manager and tools.

So... if any Arch user wants to run backintime, he/she can download the files I maintain from here. Once downloaded, the package is built by that same user.

I hope this makes sense.

@buhtz
Copy link
Member

buhtz commented Feb 4, 2024

So AUR packages are not pre-build? Every user need to run the build procedure by them self?

What is the advantage using AUR compared to build and install from upstream? Just want to understand.

Then why you (as package AUR maintainer) not just remove the "make test" from that procedure? It would save time, resources and a lot of CO2. It is IMHO unusual to have regular users running unit tests.

@graysky2
Copy link
Contributor

graysky2 commented Feb 4, 2024

So AUR packages are not pre-build? Every user need to run the build procedure by them self?

Correct

What is the advantage using AUR compared to build and install from upstream?

Advantage is that all dependencies and files are handled by the Arch package manager pacman so everything is clean/no orphaned files, etc.

Then why you (as package AUR maintainer) not just remove the "make test" from that procedure?

This is an option. The check function is optional, but custom is to include it if upstream offers one. I don't know what the official policy is for including it or not including it.

Note that users can also build with the --nocheck option to skip it or they can manually comment it out before building. To be clear, the files in the AUR right now build just fine. All deps and make-only-deps are managed by the build util.

@buhtz
Copy link
Member

buhtz commented Feb 4, 2024

Thanks for making this clear.
I am not into Arch or AUR but IMHO there should be a policy to exclude or deactivate unit tests by default. Would save a lot of resources. Beside "resources" I see no value in it. This is not what unit tests are invented for to be run by every user on every productive system. take into account that currently our unit tests do write to the real filesystem (instead of using pyfakefs for a virtual filesystem). I am not sure if every user would like that if the would be aware of that.
But I have enough other "should" in my Debian universe. 🤣

buhtz added a commit that referenced this issue Feb 13, 2024
The PyLint unit test is executed only when PyLint is installed. Exception: On TravisCI the test is executed always.

Fix #1634
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants