WithSecureOpenSource / pytest-rts

Coverage-based regression test selection (RTS) plugin for pytest
Apache License 2.0
7 stars 3 forks source link

pytest-cov integration: execution of new tests #92

Closed matveypashkovskiy closed 3 years ago

matveypashkovskiy commented 3 years ago

As agreed let's integrate with pytest-cov and move with small steps with a limited scope of PRs to do reviews faster. @guotin three "features" were suggested before:

  1. all the new tests (both committed and uncommitted)
  2. all the changed tests (both committed and uncommitted)
  3. all tests that are related to the changed source lines

And as you said, from pytest-cov pp. 2 and 3 are the same. Let's select the simplest functionality that we can build fast. For example, it could be part of p 1.: running all uncommitted tests. How does it sound? Do you see any simpler functionality?

I suppose we need a snapshot(copy) of coverage database to find tests that are not listed there thus, root folder will contain .coverage.[COMMIT_HASH] (any name could be used) file and a flag --rts-coverage-db=.coverage.[COMMIT_HASH] will be passed.

From the technical point of view let's:

guotin commented 3 years ago

I have now built a prototype that has a functionality like so (I thought this was the easiest and fastest way to showcase the idea):

  1. Run all tests with pytest --rts --cov=.-> creates a .coverage file -> rename it to rts-coverage-file.COMMITHASH
  2. Second phase options:
    • Run pytest --rts --cov=. -> runs working directory changes -> creates a new .coverage-file for that run -> can be later combined with coverage tool to produce a combined report without messing up the original file. Report combination in pytest-cov itself takes more work.
    • Run pytest --rts --committed --cov=. -> runs committed changes (current HEAD vs hash in filename)-> creates a new .coverage-file for that run -> can be later combined with coverage tool to produce a combined report without messing up the original file. Report combination in pytest-cov itself takes more work.
    • Both working directory and committed changes consider newly added tests but it also requires tweaks. You are absolutely correct how the new test discovery could be done.
      1. All useless code removed like database interactions. Also for this fast prototype, I dropped the logging of warnings for newly added lines and prioritization of tests based on previous runtimes. They might be very hard to implement in this version.
      2. The flag option could also provide the specific commithash which to compare, I tested it a bit and it requires verification of the provided commithash also existing and now went for a simpler approach.

So the status at the moment is so that I've briefly manually tested the simple use cases. I haven't gotten to altering the test code but I think I could make the integration tests pass without modifying logic. It also requires a .coveragerc file with the following to work:

[run]
relative_files = True
omit = */tmp/*
       */.venv/*

My plan now would be to make the tests work to verify my thinking. What do you think of this approach? Should I push my current prototype code here?

alexey-vyskubov commented 3 years ago

So far it sounds reasonable, please do push your prototype code. However, I must notice that the need to omit things (especially .venv!?) looks fishy.

alexey-vyskubov commented 3 years ago

How you normally do things: you run pytest ... --cov=<subdir-where-my-code-is>; there should be no need to omit the things then, right?

matveypashkovskiy commented 3 years ago

That sounds great. Could you push the branch without creating a PR. I really would like to make the scope of the feature very limited.

Run all tests with pytest --rts --cov=. -> creates a .coverage file -> rename it to rts-coverage-file.COMMITHASH

Is correct and needed but I suppose --cov=. should be replaced to --cov=pytest_rts

From the "second phase options:" let's focus only on new uncommitted tests and have integration tests for that?

I understand that the calculation of "merged" coverage is not a trivial task and maybe we don't need it at least now. Also, I agree about newly added lines - let's think about it later.

guotin commented 3 years ago

I pushed my branch here for viewing. Everything in pytest_rts/tests/ is untouched so tests are broken. And probably pylint is very unhappy. I'll work on them next.

I think you are right about the omitting. The coverage file didn't contain any junk when I retested with flask like pytest --rts --cov=.. So it's probably not required. The coverage combination is quite easy after RTS has run. By having Coverage.py >= 5.5 you could run coverage combine --keep --append rts-cov-file new-cov-file and then run coverage report. This would then be the report for the rts run and keep the original file untouched. But doing this inside the report that pytest-cov produces is another task.

matveypashkovskiy commented 3 years ago

Good. Now let's remove all the code and tests that are not related to "running new uncommitted tests". I suppose it is:

guotin commented 3 years ago

The branch now has another commit which removes everything else but running tests for new uncommitted tests. Usage:

pytest --cov=[path_to_code] --cov-context=test
mv .coverage example.sha
# add new tests as new files and as new test methods to existing files
pytest --rts --rts-coverage-db=example.sha
# only new tests are executed

Note that the database file is .coverage instead of the configuration file .coveragerc. Also if the initial coverage file is renamed to .coverage.SHA and pytest --rts --rts-coverage-db=.coverage.SHA --cov=[path_to_code] is run, then the .coverage.SHA is removed because pytest-cov seems to automatically combine files that are like .coverage.*

Now that the pytest-cov fully initializes the mapping database, the testnames have some extra identifiers that needs parsing. It triples the amount of test function names in the database compared to the original version. I don't know if this causes any issues for big projects.

So what are the next steps here? Are these pytest-rts 2.x commits to be merged into some different branch after review or what is the procedure here.

matveypashkovskiy commented 3 years ago

It looks soooo goood and clean!

So the next steps are to:

and merge it to master - thus new 2.x version should be published

matveypashkovskiy commented 3 years ago

Nice: image

matveypashkovskiy commented 3 years ago

Also, please add coverage threshold in a similar way