Closed Bug 1615330 Opened 2 years ago Closed 2 years ago

Support test filtering besides manifest filtering

Categories

(Tree Management :: Treeherder: Frontend, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ahal, Assigned: armenzg)

References

Details

(Whiteboard: [smart-sched])

Attachments

(3 files)

Armen recently added the ability to filter by test path. However it only supports directories and/or manifests at the moment. If you pass the full path to a file it won't work.

The main issue is that we don't know which manifests the test file belongs to, this information isn't encoded in the manifests-by-task.json artifact.

We should fix this.

Depends on: 1615333
Assignee: nobody → armenzg

Looking into this.

Status: NEW → ASSIGNED

Here's another push that uses a single test-info.json.gz artifact:
https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=7cb9c178201729f0aab9cd48fb025567aab317f2

The format is:

{
    "tests-by-manifest": {"<manifest name>": ["<test>", ...]},
    "manifests-by-task": {"<task label>": ["<task>", ...]}
}

Let me know if you like that better, or if you want the data structured in some other way.

Flags: needinfo?(armenzg)

I will leave the NI so I can look at it by tomorrow.

NOTE: This is WIP.

I think the structure I want to end up with is this:

{
    "test-linux1804-64-shippable/opt-mochitest-devtools-chrome-e10s-1": [
        "test path 1"
    ]
}

This is the code I have some concerns of perf impact on the frontend since it will get executed for every push loaded in the frontend

          jobTypeNameToTestPaths[jobTypeName] = tests.reduce(
            (testsWithBasePath, test) =>
              testsWithBasePath.concat(`${basePath}/${test}`),
            [],
          );

Again, this is just a concern that I don't yet know how much of an impact it will have. So let's see how we fair once I have it working.

If you're curious this is the WIP:
https://github.com/mozilla/treeherder/pull/6120/files#diff-27afdf1baa888ea21beb4b2ea894e999R198-R211

Ok, keep in mind that that format will result in a much much much larger data structure, since we will be duplicating every single test across every single task (whereas having them in two separate artifacts means we only list out the test paths once).

It's possible that gzipping means the download size isn't much bigger than the two combined.. I can do a quick test to see. We'll also need the other artifacts in addition as Marco's use case just wants manifests and doesn't care about tests.

Just a thought related to saving client resources.. could the download + processing happen on demand? E.g maybe only once the "Test path" filter is clicked but before the path is entered into the search box. The time it takes to type out a path should still be enough to do most of the processing.

A gzipped artifact with the format from comment 4 is 4.9 MB. So I'm going to go out on a limb and say the download of this artifact will completely erase any computational benefit :p

Thanks so much for checking!
Two files is fine.

Doing when that UI is triggered might make things more complicated. I will take it into consideration if perf is not good.

FYI on Thursday, I will deploying the support for Gzip compressed files. The code is on stage.

Test filtering convo:

  • full test path
  • directory
  • full manifest path (less important)
Attached file GitHub Pull Request
Flags: needinfo?(armenzg)

I've asked ahal to push another push because tasks on try are deleted after a month.

The current PR under review is only fixing the full test path method. I will add the other two once I have some time again.

This is now on staging.
Going live tomorrow.

I've added this change to fetch the artifacts only if test_paths is used:
https://github.com/mozilla/treeherder/commit/7148f76703c360db7609e89e39bd289edae4c799

Summary: Support files in the "test path" filter → Support test filtering besides manifest filtering

This is merged to master.
We will get this deployed later today.

Live.

Also added support for manifests.

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

When I use the filter by test_paths, I get no jobs. But when I reload the page, then they come in. We are only loading the path files on page load in componentDidUpdate of Push.jsx. We should also load them on a filtering change in handleUrlChanges. As well as, in that function, evaluate if test_paths is no longer in the url, then empty those strings.

You need to log in before you can comment on or make changes to this bug.