Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create symlink pointing to workdir of the last test run #1549

Closed
wants to merge 1 commit into from

Conversation

lkotek
Copy link
Contributor

@lkotek lkotek commented Sep 30, 2022

Minor change intended to simplify checking of logs from the last run (e.g. in case with large number of test runs is present).

@lkotek lkotek force-pushed the lkotek-last-symlink branch 2 times, most recently from 3abda14 to 468c2bc Compare October 4, 2022 10:56
Signed-off-by: Lukas Kotek <lkotek@redhat.com>
@psss
Copy link
Collaborator

psss commented Aug 7, 2023

Sorry for the late response. Seems this pull request slipped through the cracks. So the main motivation of the newly added symlink is to make it easy to check the last executed run. Isn't this a bit duplicating the --last option? For example:

tmt run --last report

Also, in order to check the log of the (last) running job the --follow option can be used. Unfortunately this is currently broken due to a recent regression but should be fixed by #2248:

tmt run --last --follow
tmt run --id 123 --follow

Btw, the --last option is also implemented using a symlink and suffers from the problem when multiple runs are executed in parallel, see #525. Thus I'm not sure whether the new symlink under /var/tmp/tmt is the right approach.

@lukaszachy, @happz, thoughts?

@happz
Copy link
Collaborator

happz commented Aug 8, 2023

What would be the difference between the last-run symlink created by tmt.utils.Config and this new symlink? They seem to be doing the same job AFAICT.

Yes, the race condition is still there, we'd have to use a locking file to synchronize all involved processes, but it should be doable (and worth the effort, I'd say, it shouldn't be hard and it would make the feature behave correctly).

@lkotek
Copy link
Contributor Author

lkotek commented Aug 14, 2023

Sorry for the late response. Seems this pull request slipped through the cracks. So the main motivation of the newly added symlink is to make it easy to check the last executed run. Isn't this a bit duplicating the --last option?

I also have to sorry for my response :-/ Yes, you're right about the motivation. It seemed to me like something I would use that time (it still does).

Regarding --last option, I am aware of that. I consider this new functionality to be complementary to it. You can easily reach the latest result just via knowing the expected path (and can be simply embedded into some script executing tmt run etc) without necessity of triggering new tmt run.

Btw, the --last option is also implemented using a symlink and suffers from the problem when multiple runs are executed in parallel, see #525. Thus I'm not sure whether the new symlink under /var/tmp/tmt is the right approach.

That's true, this was about to address more basic scenarios other than parallel execution. Regarding where to place symlink, I am completely opened to ideas here. I don't have any strong preference, it just seemed logical to me.

@lkotek
Copy link
Contributor Author

lkotek commented Aug 14, 2023

What would be the difference between the last-run symlink created by tmt.utils.Config and this new symlink? They seem to be doing the same job AFAICT.

The motivation here was that symlink is created no matter any special option like --last is passed to tmt command. It is simply created for every single run of tmt without necessity of thinking about getting the last run before.

@psss
Copy link
Collaborator

psss commented Oct 6, 2023

The motivation here was that symlink is created no matter any special option like --last is passed to tmt command.

Actually, that is exactly the case of the symlink created by tmt.utils.Config, the symlink is created always:

> tmt run discover
/var/tmp/tmt/run-162

/plans/example
    discover
        how: shell
        summary: 1 test selected

> ls -l ~/.config/tmt/
celkem 12
-rw-rw-r--. 1 psss psss  61 16. led  2020 cli.fmf
lrwxrwxrwx. 1 psss psss  20  6. říj 18.20 last-run -> /var/tmp/tmt/run-162
-rw-rw-r--. 1 psss psss 652  4. dub  2022 tmt-complete.bash

Seems to duplicate existing functionality, closing. Feel free to reopen of you don't agree.

@psss psss closed this Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants