log2timeline / dfdatetime

Digital Forensics date and time
Apache License 2.0
23 stars 15 forks source link

clean unused test dependencies #246

Closed bnavigator closed 1 year ago

bnavigator commented 1 year ago

Since the drop of Python 2, six, mock and pbr are not used anywhere. Remove them from the test dependencies. It already confused a packager.

joachimmetz commented 1 year ago

Thx for flagging, some of these files are generated, I'll have a look to update the generator first. yeah packagers do strange things, since there is no need to package the tests.

bnavigator commented 1 year ago

No need to package. But use them in order to make sure that the package works.

joachimmetz commented 1 year ago

No need to package. But use them in order to make sure that the package works.

these are unit tests not deployment tests, so they give you no guarantee about this

bnavigator commented 1 year ago

Of course, but it is better than nothing.

https://en.opensuse.org/openSUSE:Packaging_Python#Running_tests

joachimmetz commented 1 year ago

But you're running these during packaging, not on the packaged results, so they are basically testing if you can run the unit test on the build systems and not if you will "distribute something that does not work at all".

joachimmetz commented 1 year ago

Changed the generator and corresponding changes in https://github.com/log2timeline/dfdatetime/pull/247

bnavigator commented 1 year ago

But you're running these during packaging, not on the packaged results

That is not correct. "We" run the tests on the packaged results, but that is not what this PR or the linked mailinginlist post is about.

joachimmetz commented 1 year ago

That is not correct. "We" run the tests on the packaged results, but that is not what this PR or the linked mailinginlist post is about.

Correct this is outside the scope of this PR but isn't %check run before the rpm is created? so any changes tests could make to the "installed" (pre packaged) files are included in the package (rpm)?

bnavigator commented 1 year ago

Yes, but the checks are run on the %buildroot which is where all the files have been installed by %install and are packaged from in a later state. Again, this has nothing to do with this PR. The commands in the specfile %check section do not run run_tests.py but use unittest directly.

so any changes tests could make to the "installed" (pre packaged) files are included in the package (rpm)?

That would be the case. Although changes to the %buildroot are only allowed in %install, not %check. PYTHONDONTWRITEBYTECODE=1 is set and who changes source files in unit tests? That sounds dumb.

joachimmetz commented 1 year ago

who changes source files in unit tests?

Consider someone with malicious intent.

bnavigator commented 1 year ago

Well that's on the packager. I don't see much difference in the source code itself and the source code for tests. Malicious code could be in both.

joachimmetz commented 1 year ago

true, but it is an additional attack surface or surface for additional errors, but agree that is the responsibility of the packager/distribution