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

Add test guidelines #1129

Open
lukpueh opened this issue Sep 10, 2020 · 8 comments
Open

Add test guidelines #1129

lukpueh opened this issue Sep 10, 2020 · 8 comments
Labels
discussion Discussions related to the design, implementation and operation of the project documentation Documentation of the project as well as procedural documentation

Comments

@lukpueh
Copy link
Member

lukpueh commented Sep 10, 2020

The contribution docs say, new software features or changes must be unit tested, but the current test suite is a mix of unit-, integration-, system-, regression-, etc. tests.
It would be helpful to better structure different types of tests in the test suite, and to add some guidelines for testing.

Personally, I have chosen a pragmatic non-SWE approach, a la
"Tailor your tests towards checking the desired functionality of your code and not the coverage report!

@lukpueh lukpueh added the discussion Discussions related to the design, implementation and operation of the project label Sep 10, 2020
@lukpueh lukpueh changed the title Discuss/Add test guidelines Add test guidelines Sep 10, 2020
@lukpueh lukpueh added the documentation Documentation of the project as well as procedural documentation label Sep 10, 2020
@lukpueh
Copy link
Member Author

lukpueh commented Sep 10, 2020

@jku kindly provided some insights about test metadata in #870 (comment). They might be a good fit for a test guideline.

@trishankatdatadog
Copy link
Member

I personally find CodeCov to be next to useless

@MVrachev
Copy link
Collaborator

MVrachev commented Nov 13, 2020

I will add to this issue to remove the requirement (which is seen in test_updater.py) to use this format for test names:
"test_" + <id of the test in the file> + "_" + <function_to_be_tested_" .

Probably, they have decided to use that pattern before to have an alphabetical order of the tests in the order they are in updater.py, but we have multiple tests with the name test_2_*, test_1_* and test_X_* as a whole.

It's really annoying when you have to add a single test which is in the middle of the test file sequence how you have to change all test names below yours. Even more annoying is you are unlucky to place your test at the begging of the test sequence...

@jku
Copy link
Member

jku commented Nov 13, 2020

the test_1_... style is typically used to achieve a specific order for the tests to run in (you might want that because the tests are borken so they require state from previous tests, but more often just because you want to bail out early if some fast preliminary tests fail and not spend time in slower more complex tests). This ordering actually fails in tuf because unittest of course sorts alphabetically: "test_10_" is run before "test_1_".

I'd say there's probably no reason to use that numbering for new tests -- and there's no problem in multiple tests having the same number: the point is usually just to define a rough order (so all "test_1_..." get executed before all "test_2_...").

@MVrachev
Copy link
Collaborator

I'd say there's probably no reason to use that numbering for new tests -- and there's no problem in multiple tests having the same number: the point is usually just to define a rough order (so all "test_1_..." get executed before all "test_2_...").

Still, I think this is unnecessary and confusing and it's better if it's removed.
We don't need to execute our tests in a particular order.

@MVrachev
Copy link
Collaborator

If the others agree with this, I can push a pr to rename the tests?

lukpueh added a commit to lukpueh/code-style-guidelines that referenced this issue Dec 1, 2020
We want to keep the style guide as minimal as possible.
Recommendations about code testing should go into a separate
document (see theupdateframework/python-tuf#1129).
@lukpueh
Copy link
Member Author

lukpueh commented Dec 1, 2020

See lukpueh/code-style-guidelines@57ed297 and secure-systems-lab/code-style-guidelines#21 (comment) pp. for basic recommendations that used to be in the code style guide.

lukpueh added a commit to lukpueh/code-style-guidelines that referenced this issue Dec 1, 2020
We want to keep the style guide as minimal as possible.
Recommendations about code testing should go into a separate
document (see theupdateframework/python-tuf#1129).
@MVrachev
Copy link
Collaborator

MVrachev commented Nov 10, 2021

We should document that we use with self.assertRaises instead of self.assertRaises as decided in #1660.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussions related to the design, implementation and operation of the project documentation Documentation of the project as well as procedural documentation
Projects
None yet
Development

No branches or pull requests

4 participants