QA (public) » openQA Project (public)

I've already suspected it but we cannot load needles that easily, e.g. when entering setDiffScreenshot("https://github.com/os-autoinst/os-autoinst-needles-openQA/blob/master/desktop-runner-super_big_and_ugly-20200428.png?raw=true"); in the console we get Uncaught DOMException: The operation is insecure. - probably because this cross-server request is now allowed. Probably an iframe would work instead but we're using some canvas features so this is probably not the way. So we really make this go though our server. I'll try out https://github.com/os-autoinst/openQA/pull/5175 for this tomorrow.

I haven't updated the PR yet but I'm still working on it on https://github.com/os-autoinst/openQA/compare/master...Martchus:openQA:needles_from_casedir-wip.

Still TODO:

1. Automated UI tests
2. One existing UI test still needs to be adapted (it also fails on https://github.com/os-autoinst/openQA/pull/5175 without any of my changes but passes on master)
3. Add a cleanup that makes sense (probably it is sufficient to just invoke find to delete everything older than X days)

A few questions came up:

1. Should this affect the needle editor? It makes totally sense for the candidates shown on the test details page but I'm not sure about the needle editor. When creating a new needle one might want to base it on the latest version of an existing needle. So probably this should at least be configurable.
2. Should the location to cache needles be configurable independently of OPENQA_BASEDIR? So far I made it just use $OPENQA_BASEDIR/webui/cache/needle-refs (we already use $OPENQA_BASEDIR/webui/cache for other files). Judging by the available disk space under that directory on OSD and o3 I think this location will be sufficient for now.

mkittler wrote in #note-10:

[...]
I can create new tickets if that's wanted.

Yes, please. Because it looks to me feasible to continue with the proposed approach we should do that.

I wouldn't merge the PR without having at least 1. implemented (our patch coverage checks complain anyway) and wouldn't enable it in production without having at least also 2. and 4. implemented.

Agreed.

For the second goal: I think this is really only relevant for the diff-view on the test details page and to some extend for the needle editor (see points above). The admin page to manage needles isn't relevant. We could of course track the version in the last match/seen statistics but I think this is not important for now.

I think the "last match/seen statistics" could still be quite helpful. If you don't mind please create an according ticket to capture that idea and cover the rough ideas what you consider necessary for that implementation.

Do you have an actual spike solution that you can demo or validation of the overall concept?

Do you have an actual spike solution that you can demo or validation of the overall concept?

I demoed the feature in the unblock today. It definitely needs some tweaking besides what I have already mentioned:

• It would probably be desirable if it also worked if CASEDIR is not pointing to a Git repository.
• It didn't work with a MicroOS test, probably due to symlinking. Maybe that is why the initial change by Scoot avoided using realpath but that was unfortunately breaking tests and compatibility with existing databases so I had to change it back (and we need to figure out something different).

But we also raised an additional concern: This relies on GitHub allowing to fetch refs from all forks of a repository just using the original repository as remote (for being able to display also needles of the correct version when a test ran on a fork a the needles repo). But this definitely works so I guess it shouldn't be a problem in practice, e.g. the following works:

$ git clone git@github.com:os-autoinst/openQA.git openQA-2
Klone nach 'openQA-2'...
remote: Enumerating objects: 101222, done.
remote: Counting objects: 100% (101219/101219), done.
remote: Compressing objects: 100% (28232/28232), done.
remote: Total 101222 (delta 70297), reused 100477 (delta 70022), pack-reused 3
Empfange Objekte: 100% (101222/101222), 55.43 MiB | 11.74 MiB/s, fertig.
Löse Unterschiede auf: 100% (70297/70297), fertig.
[martchus@linux-9lzf repos]$ cd openQA-2
$ [master] git log  401d22471af67637aa1a6ef8859f16ad34362ce3
Schwerwiegend: bad object 401d22471af67637aa1a6ef8859f16ad34362ce3
$ [master] git fetch origin 401d22471af67637aa1a6ef8859f16ad34362ce3
remote: Enumerating objects: 75, done.
remote: Counting objects: 100% (75/75), done.
remote: Compressing objects: 100% (25/25), done.
remote: Total 75 (delta 50), reused 69 (delta 50), pack-reused 0
Entpacke Objekte: 100% (75/75), 37.37 KiB | 1.13 MiB/s, fertig.
Von github.com:os-autoinst/openQA
 * branch                401d22471af67637aa1a6ef8859f16ad34362ce3 -> FETCH_HEAD
$ [master] git log  401d22471af67637aa1a6ef8859f16ad34362ce3
shows log

(The commit "401d22471af67637aa1a6ef8859f16ad34362ce3" is currently only present on my fork so https://github.com/os-autoinst/openQA/commit/401d22471af67637aa1a6ef8859f16ad34362ce3 currently shows the warning "This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.".)

Of course this could be a problem if GitHub decides to change the way they're handling forks internally. Then we had to make it add and use remotes according to the URLs in the CASEDIR and NEEDLES_DIR.

Another concern would be running into GitHub's rate limiting because we are probably going to invoke git fetch … a lot.

@tinita also brought up another idea: We save candidate images/JSON as part of the test results. I'm not sure how well that would scale. We'd avoid potential trouble with GitHub and we would probably be able to use our existing de-duplication logic for images. However, the list of candidates can be long so storing the additional images/JSON per test might still add up.

I can still create follow-ups because this is probably still the most promising approach.

When trying to create another ticket I also noticed that this ticket is actually having "os-autoinst-distri-example" in its title. That's a lot different than the title 'Show version of candidate needles the test ran on in the "diff view" on the test test details page' I was coming up with.

In fact, this whole PR still relies on having the Git repository cloned on the web UI. If one would specify a test distribution or needles directory not present under /var/lib/openqa/share/tests (like "os-autoinst-distri-example") it doesn't work. To make that work we would actually have to clone a whole new and as far as the web UI host is concerned "arbitrary" repo. So I guess that another missing piece - and actually a rather big one. Maybe it makes sense to split the part to make it work with arbitrary Git repos into yet another ticket.