https://progress.opensuse.org/https://progress.opensuse.org/themes/openSUSE/favicon/favicon.ico?15829177842021-06-25T19:11:12ZopenSUSE Project Management ToolopenQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4205442021-06-25T19:11:12Zokurzokurz@suse.com
<ul><li><strong>Category</strong> set to <i>Regressions/Crashes</i></li><li><strong>Target version</strong> set to <i>Ready</i></li><li><strong>Parent task</strong> set to <i>#94750</i></li></ul> openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4215772021-06-29T13:51:22Zmkittlermarius.kittler@suse.com
<ul></ul><blockquote>
<p>If the needles subdirectory contains needles not present on the instance, then openQA will not find them.<br>
Setting NEEDLES_DIR to needles/ on the other hand does not work.</p>
</blockquote>
<p>We need to assume by default that needles are <em>not</em> part of the Git repository referenced by a Git URL specified via <code>CASEDIR</code>. That's simply because for our openSUSE test distribution we keep needles separately. Hence it is expected that specifying a custom <code>CASEDIR</code> still loads default needles and any relative paths are also referring to the default needles dir. I've been adding an explanatory comment to the relevant code: <a href="https://github.com/os-autoinst/openQA/blob/master/lib/OpenQA/Worker/Engines/isotovideo.pm#L369" class="external">https://github.com/os-autoinst/openQA/blob/master/lib/OpenQA/Worker/Engines/isotovideo.pm#L369</a></p>
<p>If your custom <code>CASEDIR</code> repo already contains needles it should however work to load them if <code>NEEDLES_DIR</code> is empty/absent and needles are located under <code>$PRODUCTDIR/needles</code>.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4215862021-06-29T14:06:14Zmkittlermarius.kittler@suse.com
<ul></ul><p>Looks like the case mentioned in the last paragraph of my last comment is indeed broken as we set the <code>NEEDLES_DIR</code> here explicitly to <code>needles</code> which is a symlink to the default. However, as stated in the commit message of 1d8a079b731f5a88f9bd22fe03233ca6a77fc6e6 this is also required in some cases:</p>
<blockquote>
<p>When only changing the behavior for PRODUCTDIR the needles could not be<br>
loaded anymore (unless NEEDLES_DIR is specified explicitly) because the<br>
checkout of CASEDIR does not necessarily contain needles. To fix this, the<br>
symlinking of the needles directory is done in that case. See the verbose<br>
comments within the code for further explanations.</p>
</blockquote>
<p>So not setting <code>NEEDLES_DIR</code> here and assuming <code>CASEDIR</code> contains needles would break the case where it doesn't.</p>
<hr>
<p>For the sake of not breaking other use-cases (and compatibility in general) it would make sense to add another variable to specify what <code>NEEDLES_DIR</code> is relative to, e.g. <code>NEEDLES_DIR_BASE</code>. If set to <code>casedir</code> it would work like you expect it to work and if set to <code>default</code> it would be relative within the default needles directory (like it behaves currently).</p>
<p>(Note that having the needles within the test distribution directory would likely be the better default. However, I suppose it is better not to change this in an incompatible way at this point.)</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4261632021-07-13T10:05:08Zilausuchilausuch@suse.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/426163/diff?detail_id=403957">diff</a>)</li><li><strong>Category</strong> changed from <i>Regressions/Crashes</i> to <i>Feature requests</i></li></ul><p>We considered during the daily that this if a feature because:</p>
<ul>
<li>We don't consider this a regression (see <a href="https://progress.opensuse.org/issues/94735#note-3" class="external">https://progress.opensuse.org/issues/94735#note-3</a>)</li>
</ul>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4262022021-07-13T10:10:13Zmkittlermarius.kittler@suse.com
<ul><li><strong>Category</strong> changed from <i>Feature requests</i> to <i>Regressions/Crashes</i></li></ul><p><a class="user active user-mention" href="https://progress.opensuse.org/users/33662">@dancermak</a> To me it seems important not to break other use-cases (where changing <code>CASEDIR</code> shouldn't affect <code>NEEDLES_DIR</code>). It would be good to know if my suggestion would work for you.</p>
<p>Maybe someone also has a better idea for making the behavior configurable.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4262082021-07-13T10:11:17Zmkittlermarius.kittler@suse.com
<ul><li><strong>Category</strong> changed from <i>Regressions/Crashes</i> to <i>Feature requests</i></li></ul> openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4270032021-07-14T09:09:33Zlivdywanliv.dywan@suse.com
<ul><li><strong>Due date</strong> set to <i>2021-08-06</i></li><li><strong>Status</strong> changed from <i>New</i> to <i>Feedback</i></li><li><strong>Assignee</strong> set to <i>mkittler</i></li></ul><p>Marius agreed to take the ticket while we're waiting for clarification</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4324262021-08-02T09:56:14Zdancermakdcermak@suse.com
<ul></ul><p>mkittler wrote:</p>
<blockquote>
<p>Looks like the case mentioned in the last paragraph of my last comment is indeed broken as we set the <code>NEEDLES_DIR</code> here explicitly to <code>needles</code> which is a symlink to the default. However, as stated in the commit message of 1d8a079b731f5a88f9bd22fe03233ca6a77fc6e6 this is also required in some cases:</p>
<blockquote>
<p>When only changing the behavior for PRODUCTDIR the needles could not be<br>
loaded anymore (unless NEEDLES_DIR is specified explicitly) because the<br>
checkout of CASEDIR does not necessarily contain needles. To fix this, the<br>
symlinking of the needles directory is done in that case. See the verbose<br>
comments within the code for further explanations.</p>
</blockquote>
<p>So not setting <code>NEEDLES_DIR</code> here and assuming <code>CASEDIR</code> contains needles would break the case where it doesn't.</p>
<hr>
<p>For the sake of not breaking other use-cases (and compatibility in general) it would make sense to add another variable to specify what <code>NEEDLES_DIR</code> is relative to, e.g. <code>NEEDLES_DIR_BASE</code>. If set to <code>casedir</code> it would work like you expect it to work and if set to <code>default</code> it would be relative within the default needles directory (like it behaves currently).</p>
<p>(Note that having the needles within the test distribution directory would likely be the better default. However, I suppose it is better not to change this in an incompatible way at this point.)</p>
</blockquote>
<p>mkittler wrote:</p>
<blockquote>
<p>Looks like the case mentioned in the last paragraph of my last comment is indeed broken as we set the <code>NEEDLES_DIR</code> here explicitly to <code>needles</code> which is a symlink to the default. However, as stated in the commit message of 1d8a079b731f5a88f9bd22fe03233ca6a77fc6e6 this is also required in some cases:</p>
<blockquote>
<p>When only changing the behavior for PRODUCTDIR the needles could not be<br>
loaded anymore (unless NEEDLES_DIR is specified explicitly) because the<br>
checkout of CASEDIR does not necessarily contain needles. To fix this, the<br>
symlinking of the needles directory is done in that case. See the verbose<br>
comments within the code for further explanations.</p>
</blockquote>
<p>So not setting <code>NEEDLES_DIR</code> here and assuming <code>CASEDIR</code> contains needles would break the case where it doesn't.</p>
<hr>
<p>For the sake of not breaking other use-cases (and compatibility in general) it would make sense to add another variable to specify what <code>NEEDLES_DIR</code> is relative to, e.g. <code>NEEDLES_DIR_BASE</code>. If set to <code>casedir</code> it would work like you expect it to work and if set to <code>default</code> it would be relative within the default needles directory (like it behaves currently).</p>
<p>(Note that having the needles within the test distribution directory would likely be the better default. However, I suppose it is better not to change this in an incompatible way at this point.)</p>
</blockquote>
<p>How about using <code>PRODUCTDIR</code> for this? If <code>PRODUCTDIR</code> is a git repo, then os-autoinst could assume that the needles are in it, but if <code>CASEDIR</code> is one, then it would not?</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4332122021-08-04T12:24:36Zmkittlermarius.kittler@suse.com
<ul></ul><p>Let me think it though:</p>
<ol>
<li>So far we don't allow setting a Git URL for <code>PRODUCTDIR</code> but that's actually good because it means we cannot break any existing behavior.</li>
<li>The referenced Git repo needed to contain <code>main.pm</code> on top-level but I suppose for your use-case (<a href="https://github.com/dcermak/kiwi-functional-tests" class="external">https://github.com/dcermak/kiwi-functional-tests</a>) that's a given.</li>
<li>If <code>PRODUCTDIR</code> is a relative (or even absolute) path we should likely not change the behavior because a relative <code>PRODUCTDIR</code> (or absolute if <code>ABSOLUTE_TEST_CONFIG_PATHS</code> is set) is assigned by default for the job's <code>DISTRI</code>/<code>VERSION</code> and it would be very weird if assigning a <code>PRODUCTDIR</code> explicitly would cause a different behavior.</li>
<li>It is potentially a little bit confusing that the behavior of the needle lookup changes if <code>PRODUCTDIR</code> is set to be a Git URL. It is not intuitive to assume that a change to <code>PRODUCTDIR</code> changes the directory a relative <code>NEEDLES_DIR</code> is considered to be part of.</li>
<li>What <code>CASEDIR</code> should be set then? Without changing anything in that regard, <code>$CASEDIR/lib</code> would still be added to Perl's include path by os-autoinst so one needed to make sure to change <code>CASEDIR</code> as well in accordance.</li>
<li>This requires changes in os-autoinst and openQA:
<ol>
<li>os-autoinst: Allow checking out Git repos when <code>PRODUCTDIR</code> is a Git URL.</li>
<li>openQA: Adjust code in <code>isotovideo.pm</code> to avoid setting <code>NEEDLES_DIR</code> when <code>PRODUCTDIR</code> is a Git URL.</li>
<li>openQA: Should we possibly just pass on the Git URL from <code>PRODUCTDIR</code> to os-autoinst as <code>CASEDIR</code>? This would maybe help with 5. and we wouldn't need 6.1.</li>
</ol></li>
</ol>
<hr>
<p>Considering 3. to 6., the more explicit <code>NEEDLES_DIR_BASE</code> approach suggested in my previous comments seems better. It is less confusing and raises less questions. Of course it has the disadvantage that we need to introduce yet another variable.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4337762021-08-09T06:19:41Zdancermakdcermak@suse.com
<ul></ul><p>mkittler wrote:</p>
<blockquote>
<p>Considering 3. to 6., the more explicit <code>NEEDLES_DIR_BASE</code> approach suggested in my previous comments seems better. It is less confusing and raises less questions. Of course it has the disadvantage that we need to introduce yet another variable.</p>
</blockquote>
<p>That makes sense, let's go with <code>NEEDLES_DIR_BASE</code> then.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4339822021-08-09T16:21:01Zlivdywanliv.dywan@suse.com
<ul><li><strong>Due date</strong> changed from <i>2021-08-06</i> to <i>2021-08-13</i></li></ul><p>It's probably fair to say that more urgent issues had priority, and this was pending open questions as well. Bumping the <em>due date</em>.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4359862021-08-17T16:06:51Zmkittlermarius.kittler@suse.com
<ul></ul><p>I've started to implement by approach but it but it isn't really straight forward. That's also because it is a bit questionable how openQA currently discards a relative <code>NEEDLES_DIR</code> completely (in favor of the default one). Even specifying a path like <code>foo/bar/baz</code> for <code>NEEDLES_DIR</code> would simply mean to just read needles from the default needles directory without considering the sub directories.</p>
<p>(See <a href="https://github.com/Martchus/openQA/pull/new/needles-dir-base" class="external">https://github.com/Martchus/openQA/pull/new/needles-dir-base</a> for my current attempt.)</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4362742021-08-18T10:44:00Zokurzokurz@suse.com
<ul></ul><p>Discussed with mkittler and cdywan. We propose to shift more logic into os-autoinst and implement an additional keyword so that os-autoinst looks up only a fallback needles repo in case needles are not found anywhere else. Also we assume that os-autoinst is easier to change and maintain. The current proposal is to use <code>NEEDLES_DIR=[<keyword>]<dir></code>, e.g. <code>NEEDLES_DIR=[fallback]my/productdir/needles</code> for a relative fallback directory. The only suggested to be supported keyword should be "fallback" for now.</p>
<p>Suggested steps:</p>
<ul>
<li>In os-autoinst parse keyword from <code>NEEDLES_DIR</code></li>
<li>If "[fallback]" then try to find needles in all default paths, e.g. subdir of casedir, productdir, etc., only if not found look up in the fallback one</li>
<li>Prepare setting fallback in openQA Worker/Engines/isotovideo.pm but do not merge yet</li>
<li>Deploy new feature in os-autoinst</li>
<li>Wait grace period</li>
<li>Deploy openQA feature</li>
</ul>
<p>I put the above ideas into a new ticket <a class="issue tracker-4 status-1 priority-3 priority-lowest child" title="action: Support relative needle directories together with tests checked out from git (New)" href="https://progress.opensuse.org/issues/97112">#97112</a>. To conclude the current story quicker as we exceeded our due-date I suggest to just document the current best workaround as "keep in mind" in <a href="http://open.qa/docs/#_triggering_tests_based_on_an_any_remote_git_refspec_or_open_github_pull_request" class="external">http://open.qa/docs/#_triggering_tests_based_on_an_any_remote_git_refspec_or_open_github_pull_request</a> and resolve the ticket</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4362862021-08-18T10:55:01Zokurzokurz@suse.com
<ul><li><strong>Copied to</strong> <i><a class="issue tracker-4 status-1 priority-3 priority-lowest child" href="/issues/97112">action #97112</a>: Support relative needle directories together with tests checked out from git</i> added</li></ul> openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4363102021-08-18T11:20:07Zmkittlermarius.kittler@suse.com
<ul></ul><p>PR for extending the documentation as stated in <a class="user active user-mention" href="https://progress.opensuse.org/users/17668">@okurz</a>'s comment: <a href="https://github.com/os-autoinst/openQA/pull/4130" class="external">https://github.com/os-autoinst/openQA/pull/4130</a></p>
<hr>
<p>So I will <em>not</em> follow the <code>NEEDLES_DIR_BASE</code> approach but the feature request added by <a class="issue tracker-4 status-1 priority-3 priority-lowest child" title="action: Support relative needle directories together with tests checked out from git (New)" href="https://progress.opensuse.org/issues/97112">#97112</a> will make the use case described in this ticket work by default without affecting other use cases.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4363162021-08-18T11:29:19Zokurzokurz@suse.com
<ul></ul><p><a href="https://github.com/os-autoinst/openQA/pull/4130" class="external">https://github.com/os-autoinst/openQA/pull/4130</a> merged. Triggered <a href="https://app.circleci.com/pipelines/github/os-autoinst/openQA/7415/workflows/93acde46-2aad-45a4-b1ea-4cef970081d2/jobs/70002" class="external">https://app.circleci.com/pipelines/github/os-autoinst/openQA/7415/workflows/93acde46-2aad-45a4-b1ea-4cef970081d2/jobs/70002</a> to build updated documentation. I expect that this creates another doc update PR. As soon as that one deployed updated config I consider the ticket resolvable.</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4364632021-08-18T16:38:43Zokurzokurz@suse.com
<ul><li><strong>Status</strong> changed from <i>Feedback</i> to <i>Resolved</i></li></ul><p>open.qa/docs does not yet have it but at least <a href="https://github.com/os-autoinst/openQA/blob/master/docs/WritingTests.asciidoc#triggering-tests-based-on-an-any-remote-git-refspec-or-open-github-pull-request" class="external">https://github.com/os-autoinst/openQA/blob/master/docs/WritingTests.asciidoc#triggering-tests-based-on-an-any-remote-git-refspec-or-open-github-pull-request</a> is updated. Let's say it counts :)</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4366252021-08-19T05:53:43Zokurzokurz@suse.com
<ul></ul><p><a href="https://github.com/os-autoinst/openQA/pull/4132" class="external">https://github.com/os-autoinst/openQA/pull/4132</a> is the according official documentation change</p>
openQA Project - action #94735: needles not found in `needles` subdirectory when CASEDIR is a git repositoryhttps://progress.opensuse.org/issues/94735?journal_id=4416922021-09-02T13:20:52Zokurzokurz@suse.com
<ul><li><strong>Due date</strong> deleted (<del><i>2021-08-13</i></del>)</li></ul>