Page MenuHomeSoftware Heritage

Add hypothesis strategies and enrich WebTestCase
ClosedPublic

Authored by anlambert on Dec 14 2018, 6:22 PM.

Details

Summary

Second diff exposing the work I have done so far on improving swh-web tests.
This one is about implementing hypothesis strategies to provide relevant
input data to tests. This is still a WIP, so more strategies should be
added as I progress on rewriting the tests.

Define hypothesis strategies in order to generate relevant input data for tests.
Also, add new class methods to WebTestCase in order to retrieve tests data
in a JSON serializable format to ease tests implementation.

Related T1271

Diff Detail

Repository
rDWAPPS Web applications
Branch
tests-improvements-diff2
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3104
Build 3966: tox-on-jenkinsJenkins
Build 3965: arc lint + arc unit

Event Timeline

I don't understand the purpose of the SWHWebTestCase.*_get_* methods. They look like small wrappers around the storage. Why can't the storage be used directly?

swh/web/tests/strategies.py
48–69

It looks like these three strategies generate a single value. Shouldn't we define an auxiliary function and use it with [hypothesis.strategies.builds](https://hypothesis.readthedocs.io/en/latest/data.html#hypothesis.strategies.builds)?

Or maybe make them composite strategies that take bytes as argument and build the hashes from there?

171

Rename to something like content_sha1s_with_ctags?

197–206

I don't understand this strategy.

ardumont added inline comments.
swh/web/tests/strategies.py
48–69

An example of what you proposed would be great ;)

197–206

Antoine most probably elected this as the single revision having an ancestor relation as a building step.
And either forget to work more on this or did not find a way to do better (found other revision like this ;)

The docstring should be fixed to remove the plural on the sentence returning revisions.

swh/web/tests/testcase.py
15–16

No need to have the SWH prefix everywhere, i tried to remove those redundant prefixes in other repositories.

anlambert retitled this revision from swh-web: Add hypothesis strategies and enrich SWHWebTestCase to Add hypothesis strategies and enrich SWHWebTestCase.Dec 17 2018, 10:42 AM
anlambert added inline comments.
swh/web/tests/strategies.py
48–69

In order for the tests to not take too long to execute, I have limited the maximum number of examples generated by hypothesis to 1.
I also tested to use the from_regex and text strategy from hypothesis to generate the hashes but I ended up
having only string full of '0' (due to the restriction on the numbers of examples).

So that's why I implemented this way.

171

ack

197–206

Yes, I harcoded some revision ids for the tests input by looking at the following git log ouput:

✔ ~/dev/highlightjs-line-numbers.js [master|✔] 
11:58 $ git log --oneline --graph
*   4ff78fc Merge pull request #52 from wcoder/dev
|\  
| * 00356ff Update version
| * f4a0634 Fix whitespaces for not modified sources
| * 9029d20 Add support nohighlight
|/  
*   f1cdbc3 Merge pull request #49 from wcoder/dev
|\  
| * 35dcbb3 Update version
| *   d7c52cd Merge branch 'vai0-master' into dev
| |\  
| | *   346fb4d Merge branch 'master' of https://github.com/vai0/highlightjs-line-numbers.js into vai0-master
| | |\  
| | | * f3f53fe Delete highlightjs-line-numbers.min.js
| | | * 0081bb8 update dist
| | | *   4692cfd Merge pull request #1 from vai0/add-readyState
| | | |\  
| |_|/ /  
|/| | |   
| | | * 1b77273 add forgotten readyState
| |_|/  
|/| |   
| * |   0e411b1 Merge pull request #48 from anlambert/fix-duplicateMultilineNode-empty-lines
| |\ \  
| | |/  
| |/|   
| | * 708a35e Update highlightjs-line-numbers.js
| | * 1cf33c8 Fix output when hljs multi-line element contains an empty line
| |/  
|/|   
| *   aa5ebf1 Merge branch 'master' into dev
| |\  
| |/  
|/|   
* | e03ee42 Update issue templates
* | 5bb5a73 Create CODE_OF_CONDUCT.md
* | c0519a1 Update README.md
* | 8c67b55 Update README.md
* |   675285e Merge pull request #33 from wcoder/dev
|\ \  
| | * 1b24c7d Update version
| |/  
| * a007797 Update README.md
| * 59f905d Fix for the last line if empty
| * 352a5ba Add fix
|/  
*   4ab966b Merge pull request #30 from wcoder/bugfixes/multiline-nodes

I agree that it could be generated programmatically, will look into it.

swh/web/tests/testcase.py
15–16

Ack, I will make the changes in master first and rebase.

Update:

  • patch storage instances in WebTestCase.setupClass
  • code factorization
  • add new strategies for snapshots
  • rework [non_]ancestor_revisions strategy implementation and docstring (find relevant revisions dynamically instead of hardcoding them)

Update: Add variables in the testcase module for skipping tests if ctags
with json support or fossology-nomossa are not installed.

There are some:

  • typos that need fixing
  • some comments whose phrasing need improvements.

Then it should be good to go ;)

swh/web/tests/strategies.py
34

swh

39

swh

110

Apparently, composite does some magic here (draw is not marked as optional in the main unknown_content definition).

161

swh

244

with no ancestor relation

257

whose parents

277

test archive. Those contents are ctags compatible, that is running ctags on those lay results.

This revision now requires changes to proceed.Dec 18 2018, 2:43 PM
anlambert retitled this revision from Add hypothesis strategies and enrich SWHWebTestCase to Add hypothesis strategies and enrich WebTestCase.Dec 18 2018, 2:44 PM
anlambert edited the summary of this revision. (Show Details)
anlambert added inline comments.
swh/web/tests/strategies.py
34

well seen!

110

Quoting hypothesis doc [1]:

The decorated function has the initial argument removed from the list, but will accept all the others in the expected order. Defaults are preserved.

[1] https://hypothesis.readthedocs.io/en/latest/data.html#composite-strategies

Update: address ardumont comments

This revision is now accepted and ready to land.Dec 18 2018, 3:05 PM
This revision was automatically updated to reflect the committed changes.