Refactor/enrich the rest of Quark's tests

[haeter525]

Description

Please refer here. For the replacement of Androguard, I want to write tests to improve the test coverage of Quark. This is the final PR. (You can find the previous PR here )

In this PR, I focus on these files.

• quark/Objects/analysis.py
• quark/Objects/quarkrule.py
• quark/report.py
• quark/freshquark.py
• all seven files in quark/utils (colors.py, graph.py, pprint.py, output.py, etc.)

Code Changes

• For the existing tests: Divide them by their test scenarios.
• For the new tests: Add them according to two strategies and the coding guideline discussed in the above issue.

Files	# Tests added for normal inputs	# Tests added for error inputs	# Tests modified
test_analysis.py	1	0	0
test_quarkrule.py	1	4	-
test_report.py	4	6	-
test_freshquark.py	2	0	-
test_colors.py	1	0	-
test_graph.py	3	0	-
test_output.py	3	0	-
test_pprint.py	5	0	-
test_regex.py	11	3	-
test_tools.py	5	1	1
test_weight.py	0	0	3
Total	36	14	4

Related Discussions

1. issue Enrich Quark tests gsoc2021-ShengFengLu#1
2. Discussion Asking review for PR "Refactor/enrich the tests of the analysis part of modules" #173

[pep8speaks]

Hello @haeter525! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file tests/utils/test_graph.py:

Line 104:80: E501 line too long (113 > 79 characters)

Comment last updated at 2021-06-28 09:54:08 UTC

[krnick]

Thank you @haeter525 for the excellent work!
It seems that CI has failed the test. Could you please address it?

[haeter525]

Hi, @krnick

It looks like CI didn't provide the package graphviz, which is a requirement of Quark.

I propose to add the following lines into .travis.yml to have it installed.

before_install:
  - sudo apt-get -y install graphviz

Please take a look at the following commit. Thanks.

[codecov-commenter]

Codecov Report

Merging #189 (afe6af8) into master (5fc3688) will increase coverage by 10.40%.
The diff coverage is 98.15%.

@@             Coverage Diff             @@
##           master     #189       +/-   ##
===========================================
+ Coverage   73.58%   83.99%   +10.40%     
===========================================
  Files          35       43        +8     
  Lines        1772     2168      +396     
===========================================
+ Hits         1304     1821      +517     
+ Misses        468      347      -121     

Impacted Files	Coverage Δ
tests/utils/test_colors.py	90.00% <90.00%> (ø)
tests/utils/test_regex.py	95.23% <95.23%> (ø)
tests/test_report.py	95.55% <95.55%> (ø)
tests/Object/test_analysis.py	100.00% <100.00%> (ø)
tests/Object/test_quarkrule.py	100.00% <100.00%> (ø)
tests/test_freshquark.py	100.00% <100.00%> (ø)
tests/utils/test_graph.py	100.00% <100.00%> (ø)
tests/utils/test_output.py	100.00% <100.00%> (ø)
tests/utils/test_pprint.py	100.00% <100.00%> (ø)
tests/utils/test_tools.py	100.00% <100.00%> (ø)
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fc3688...afe6af8. Read the comment docs.

[krnick]

Why do descriptors \\(\\)V need to be escaped?

[krnick]

Nice work!
Thank you @haeter525 for helping Quark greatly improve the test coverage and consider many aspects of the test problem.

There is only one thing I am not sure that you use find_methods to search method with the escape character \ inside descriptor in file tests/utils/test_graph.py .

For example:

 analysis_object.find_methods(
		 "Ljava/lang/Object;",
		 "<init>",
		 "\\(\\)V",

In fact, it could be written like this:

 analysis_object.find_methods(
		 "Ljava/lang/Object;",
		 "<init>",
		 "()V",

Is there any reason?

[haeter525]

Hi @krnick, thank you for your review!

The reason is a problem with regular expressions.

Because find_methods expects regular expressions as its inputs, the parentheses need '\' to search the descriptor properly.

[krnick]

Because find_methods expects regular expressions as its inputs, the parentheses need '' to search the descriptor properly.

In fact, find_method(implemented by Apkinfo class) has already handled it, so there is no need to manually add escape character. I would recommand using Apkinfo(apk).find_method() to search the method.

Also, whenever you propose a new PR, please do the following command locally and make sure that every test passes.

pipenv --rm
pipenv install --skip-lock --dev
pytest

[sourcery-ai]

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 4.79%.

Quality metrics	Before	After	Change
Complexity	0.00 ⭐	0.00 ⭐	0.00
Method Length	35.50 ⭐	23.12 ⭐	-12.38 👍
Working memory	4.15 ⭐	4.06 ⭐	-0.09 👍
Quality	88.96% ⭐	93.75% ⭐	4.79% 👍

Other metrics	Before	After	Change
Lines	101	162	61

Changed files	Quality Before	Quality After	Quality Change
tests/utils/test_output.py	85.89% ⭐	91.39% ⭐	5.50% 👍
tests/utils/test_tools.py	84.60% ⭐	96.44% ⭐	11.84% 👍
tests/utils/test_weight.py	93.03% ⭐	93.43% ⭐	0.40% 👍

Here are some functions in these files that still need a tune-up:

File	Function	Complexity	Length	Working Memory	Quality	Recommendation

Legend and Explanation

The emojis denote the absolute quality of the code:

• ⭐ excellent
• 🙂 good
• 😞 poor
• ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.

---

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

[haeter525]

Hi @krnick

I've pushed a commit to change all the backslashes into the use of find_method in apkinfo.py.
Please take a look. Thank you!

[krnick]

Thanks for the patch!. LGTM