857012 - Need a way to reconcile manifest files and test_ methods

Status: RESOLVED WONTFIX

(Testing :: Mozbase, enhancement)

Description

[Zac C (:zac)]

Due to the 'inclusive' nature of the ManifestParser it relies on the users to keep the manifest files in sync with the number of test_ methods in classes. 

This is difficult and my team has occasionally found tests not running because they have slipped through the cracks. Now we have over 100 tests I don't think it reasonable to make reconciling the number of manifest listings with the test methods a manual task (also very unreliable). 

Our use case is that we will always want every test method in our repo to also be tagged in the manifest file. We would never disable a test 

I think it would be very useful to have ManifestParser help with this task, seeing as it is very close to the task already. 

In this case I don't think we want to stop the test run, but I have two ideas so far:

A. Log a *warning* message at the start of a test run, eg: "test_chickens.py was found but is not tagged in the manifest". User would see this in their test results and be prompted to resolve it.

B. A command line like pytests's --collectonly which outputs a list of tests but also will try to find and notify the user of any tests that have escaped inclusion. --list-untagged-tests or somesuch which would list .py files containing test_ methods that do not have corresponding tags in the manifest files. The user would have to run this occasionally.

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

It sounds like this could be an enhancement for manifestparser command line tool, where an existing manifest can be compared against the test discovery when passing one or more directories. As I understand it, the test runner takes either a manifest location or a directory, so unless this is changed to take both, I don't see the test runner being able to identify missed tests.

Perhaps something like:

manifestparser [options] compare manifest directory

Comment 2

[Zac C (:zac)]

Yeah that's sort of like my option B. I wish it were a bit more 'automatic' as it is it still relies on somebody to both know the command exists and to run it occasionally.

I guess the manifest file does not need to be in the same directory as the test so the test discovery could not just use the manifest directory as the root directory.

Comment 3

[Zac C (:zac)]

Just to add in a real world scenario here, we just went through and reconciled our tests manually. We only have 20 or so manifest files and 100 tests so it didn't take as long as expected but you definitely wouldn't want to do it if you have several hundred tests. Big chance of missing something.

We found two tests that haven't been run for months, the py file reference was missing from the manifest file. Also found 3 other new tests that haven't run since they were merged in a couple of weeks ago because their manifest files not included in the root manifest.

Do other projects do this task or have a shell script for it? Do they know they might be missing tests?

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I believe there are a number of projects using manifests. Mozmill has them but will not use them until we move to Mozmill 2.0. For Mozmill, an issue is not closed until we have seen the test passing in CI.

Comment 5

[Jeff Hammel]

Zac, do you mean test *files* instead of methods here?  ManifestDestiny has, AFAIK, not been used for methods to date (and, depending on what was desired, may require a bit of thinking to do so).  Assuming you're talking about files, I have a few ideas but will wait for your clarification.

Blocking on xpccheck.py upstreaming, as, I'm pretty sure, we *will* want this

Comment 6

[Zac C (:zac)]

jhammel, I hadn't really considered the distinction between the two. I really only thought of the methods that a file could not be a test, only a method is a test. I was basing my expected behaviour on the test collation/discovery of unit testing tools like python unittest which requires the file and the test to both be prefixed with test_.

I don't want to drive the spec of this as it's not really my strength and I don't understand the technical consequences but it sounds like scooping up missing files would probably be sufficient. 

I accept you've really messed up if you've got a test_ method in a file not named test_!

Comment 7

[Jeff Hammel]

I'm not 100% sure I understand comment 6, but for manifestparser.py purposes, I think we're talking about files here.

So I've tried to outline as much of a general solution as possible in bug 860499 . As voiced there and elsewhere, we do want to retain as much flexibility as (reasonably) possible as different harnesses (etc) may want to do different things with tests missing from manifests; expecting a manifest to contain all of one or more subdirectories is just one use case. This is to say that the harness, for now, will have to do something to check for this (nor am I 100% convinced that the harness should be the go-to guy for this for the "usual case").

This also requires porting the code from xpccheck.py: bug 683660

In addition, there are two features that *could* help with this:
* ability to (recursively?) include directories in manifests (?)
* more programmatic control a la command line (e.g.):  `manifestparser create`

I haven't ticketed these, as I don't know if we want them at all and they're not high priority (though neither would they be hard to do), but if you feel the need please ticket with full understanding of exactly what is desired (that is, something actionable to the existing manifestparser.py code).

Comment 8

[Geoff Brown [:gbrown]]

Mass closing bugs with no activity in 2+ years. If this bug is important to you, please re-open.