search

teemtee / tmt

Test Management Tool
MIT License
76 stars 112 forks source link

Add _all param to generate_runs #2882

Open skycastlelily opened 3 weeks ago

skycastlelily commented 3 weeks ago

Pull Request Checklist

skycastlelily commented 3 weeks ago

Fix this issue:https://github.com/teemtee/tmt/issues/2874^^

skycastlelily commented 3 weeks ago

I didn't realize run.yaml will be used for cleaning guests😅, and I am working on having try create run.yaml approach,this mr will fix the problem of removing runs from tmt try, as run.yaml will not be used for clean runs, so a dummy one works:)

On Wed, Apr 24, 2024 at 3:39 PM Miloš Prchlík @.***> wrote:

@.**** commented on this pull request.

In tmt/utils.py https://github.com/teemtee/tmt/pull/2882#discussion_r1577427084:

@@ -4618,6 +4618,9 @@ def generate_runs(

in abs_path to be generated.

invalidid = id and str(abs_childpath) != id invalid_run = not abs_child_path.joinpath('run.yaml').exists()

  • if invalid_run and abs_child_path.name.startswith('run-'):
  • os.system(f"touch {abs_child_path.joinpath('run.yaml')}")
  • invalid_run = False

I'm not sure I follow what's happening and why this helps. The commit message does very little in terms of explaining what's changing and especially why it needs to change. If a workdir lacks run.yaml, what help would it be to create a dummy, empty one? This is unclear from the PR. Can you shed more light on why a change like this solves the problem?

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#pullrequestreview-2019105028, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23FAPQ74NVL4ETSI2DDY65ORZAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJZGEYDKMBSHA . You are receiving this because you authored the thread.Message ID: @.***>

skycastlelily commented 3 weeks ago

I have created a new pull request to fix the try specific issue: https://github.com/teemtee/tmt/pull/2889^^ For general clean runs that doesn't work without the run.yaml issue,we may still need this merge request.

Is the change getmtime -> getatime intentional?

Yep,as we create the run.yaml for the runs which doesn't have that file, and the getmtime will be the time of run.yaml is created in that case. We need check the getatime,which is the time of the dir created^^

Also, if run.yaml will not be used for clean runs, does it need to exist? If we add a flag to generate_runs() to include also workdirs without run.yaml

Ah,right,we could add a flag😄and then we doesn't need to create the dummy run.yaml any more^^

On Thu, Apr 25, 2024 at 4:58 PM Miloš Prchlík @.***> wrote:

@.**** commented on this pull request.

In tmt/utils.py https://github.com/teemtee/tmt/pull/2882#discussion_r1579128732:

@@ -4618,6 +4618,9 @@ def generate_runs(

in abs_path to be generated.

invalidid = id and str(abs_childpath) != id invalid_run = not abs_child_path.joinpath('run.yaml').exists()

  • if invalid_run and abs_child_path.name.startswith('run-'):
  • os.system(f"touch {abs_child_path.joinpath('run.yaml')}")
  • invalid_run = False

I didn't realize run.yaml will be used for cleaning guests😅, and I am working on having try create run.yaml approach,this mr will fix the problem of removing runs from tmt try, as run.yaml will not be used for clean runs, so a dummy one works:)

Isn't it changing the way of discovering workdir for every user of this function? Let's say that for cleaning guests, run.yaml is not important and may not exist, but what about others? This function gets called from Status.show(), and it starts returning workdirs that were considered invalid in the past, which is not related to "cleaning guests" use case.

Also, if run.yaml will not be used for clean runs, does it need to exist? If we add a flag to generate_runs() to include also workdirs without run.yaml, does it have to be created? That might fool other parts of tmt into thinking this workdir is suddenly having apparently valid run.yaml, and therefore it's a valid workdir and should be inspected...

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#discussion_r1579128732, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23FH6LBMT6WFKXHGWRDY7DATJAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRRHE2DANJZG4 . You are receiving this because you authored the thread.Message ID: @.***>

happz commented 3 weeks ago

Is the change getmtime -> getatime intentional? Yep,as we create the run.yaml for the runs which doesn't have that file, and the getmtime will be the time of run.yaml is created in that case. We need check the getatime,which is the time of the dir created^^

"atime" is the last time the directory was "read", i.e. someone tried listing its content. It's not related to when the directory was created, that would be "ctime". But directories also have "mtime", the time when the directory was "modified", i.e. someone changed its content.

skycastlelily commented 3 weeks ago

But I checked, the output of os.path.getctime is the same as os.path.getmtime, which is the time that file is created:)

On Thu, Apr 25, 2024 at 5:43 PM Miloš Prchlík @.***> wrote:

Is the change getmtime -> getatime intentional? Yep,as we create the run.yaml for the runs which doesn't have that file, and the getmtime will be the time of run.yaml is created in that case. We need check the getatime,which is the time of the dir created^^

"atime" is the last time the directory was "read", i.e. someone tried listing its content. It's not related to when the directory was created, that would be "ctime". But directories also have "mtime", the time when the directory was "modified", i.e. someone changed its content.

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#issuecomment-2076783211, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23A7GII3BEKAYE5O5XTY7DF2BAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZWG44DGMRRGE . You are receiving this because you authored the thread.Message ID: @.***>

happz commented 3 weeks ago

But I checked, the output of os.path.getctime is the same as os.path.getmtime, which is the time that file is created:)

No, "mtime" is not the time a file (or directory) was created, it's the time it was modified. It might have been same, that may happen, see below, but it's not the same property and they will be different sooner or later. If you treat atime as "the time file/directory was created", it will bite you.

happz@multivac ~ $ mkdir /tmp/foo
happz@multivac ~ $ stat /tmp/foo
...
# just created, all times are the same
Access: 2024-04-25 12:09:44.957668243 +0200
Modify: 2024-04-25 12:09:44.957668243 +0200
Change: 2024-04-25 12:09:44.957668243 +0200
 Birth: 2024-04-25 12:09:44.957668243 +0200

happz@multivac ~ $ touch /tmp/foo/bar.txt
happz@multivac ~ $ stat /tmp/foo
...
# created a file - "modified" the directory -> access/birth did not change, modify changed
Access: 2024-04-25 12:09:44.957668243 +0200
Modify: 2024-04-25 12:10:01.693803605 +0200
Change: 2024-04-25 12:10:01.693803605 +0200
 Birth: 2024-04-25 12:09:44.957668243 +0200

happz@multivac ~ $ ls /tmp/foo
bar.txt
happz@multivac ~ $ stat /tmp/foo
...
# listed the directory - "read" the directory -> access changed, but not modify or birth
Access: 2024-04-25 12:10:22.929975365 +0200
Modify: 2024-04-25 12:10:01.693803605 +0200
Change: 2024-04-25 12:10:01.693803605 +0200
 Birth: 2024-04-25 12:09:44.957668243 +0200

happz@multivac ~ $ stat /tmp/foo/bar.txt
...
# the file was created and left untouched, all times are the same
Access: 2024-04-25 12:10:01.693803605 +0200
Modify: 2024-04-25 12:10:01.693803605 +0200
Change: 2024-04-25 12:10:01.693803605 +0200
 Birth: 2024-04-25 12:10:01.693803605 +0200

happz@multivac ~ $ echo "baz" > /tmp/foo/bar.txt
happz@multivac ~ $ stat /tmp/foo/bar.txt
...
# writing to the file - modify/change are different, not the access/creation
Access: 2024-04-25 12:10:01.693803605 +0200
Modify: 2024-04-25 12:11:33.312565817 +0200
Change: 2024-04-25 12:11:33.312565817 +0200
 Birth: 2024-04-25 12:10:01.693803605 +0200

happz@multivac ~ $ cat /tmp/foo/bar.txt
baz
happz@multivac ~ $ stat /tmp/foo/bar.txt
...
# and after reading the file, access is now different that modify and birth,
# and all three properties - ctime, atime and mtime hold different values.
Access: 2024-04-25 12:14:32.721916944 +0200
Modify: 2024-04-25 12:11:33.312565817 +0200
Change: 2024-04-25 12:11:33.312565817 +0200
 Birth: 2024-04-25 12:10:01.693803605 +0200
happz@multivac ~ $
skycastlelily commented 3 weeks ago

ah,right, thanks, professional^^, for your mentor. Luckily, we don't need to create a dummy run.yaml file any more:)

On Thu, Apr 25, 2024 at 6:19 PM Miloš Prchlík @.***> wrote:

But I checked, the output of os.path.getctime is the same as os.path.getmtime, which is the time that file is created:)

No, "mtime" is not the time a file (or directory) was created, it's the time it was modified. It might have been same, that may happen, see below, but it's not the same property and they will be different sooner or later. If you treat atime as "the time file/directory was created", it will bite you.

@. ~ $ mkdir /tmp/foo @. ~ $ stat /tmp/foo ...

just created, all times are the same

Access: 2024-04-25 12:09:44.957668243 +0200 Modify: 2024-04-25 12:09:44.957668243 +0200 Change: 2024-04-25 12:09:44.957668243 +0200 Birth: 2024-04-25 12:09:44.957668243 +0200

@. ~ $ touch /tmp/foo/bar.txt @. ~ $ stat /tmp/foo ...

created a file - "modified" the directory -> access/birth did not change, modify changed

Access: 2024-04-25 12:09:44.957668243 +0200 Modify: 2024-04-25 12:10:01.693803605 +0200 Change: 2024-04-25 12:10:01.693803605 +0200 Birth: 2024-04-25 12:09:44.957668243 +0200

@. ~ $ ls /tmp/foo bar.txt @. ~ $ stat /tmp/foo ...

listed the directory - "read" the directory -> access changed, but not modify or birth

Access: 2024-04-25 12:10:22.929975365 +0200 Modify: 2024-04-25 12:10:01.693803605 +0200 Change: 2024-04-25 12:10:01.693803605 +0200 Birth: 2024-04-25 12:09:44.957668243 +0200

@.*** ~ $ stat /tmp/foo/bar.txt ...

the file was created and left untouched, all times are the same

Access: 2024-04-25 12:10:01.693803605 +0200 Modify: 2024-04-25 12:10:01.693803605 +0200 Change: 2024-04-25 12:10:01.693803605 +0200 Birth: 2024-04-25 12:10:01.693803605 +0200

@. ~ $ echo "baz" > /tmp/foo/bar.txt @. ~ $ stat /tmp/foo/bar.txt ...

writing to the file - modify/change are different, not the access/creation

Access: 2024-04-25 12:10:01.693803605 +0200 Modify: 2024-04-25 12:11:33.312565817 +0200 Change: 2024-04-25 12:11:33.312565817 +0200 Birth: 2024-04-25 12:10:01.693803605 +0200

@. ~ $ cat /tmp/foo/bar.txt baz @. ~ $ stat /tmp/foo/bar.txt ...

and after reading the file, access is now different that modify and birth,

and all three properties - ctime, atime and mtime hold different values.

Access: 2024-04-25 12:14:32.721916944 +0200 Modify: 2024-04-25 12:11:33.312565817 +0200 Change: 2024-04-25 12:11:33.312565817 +0200 Birth: 2024-04-25 12:10:01.693803605 +0200 @.*** ~ $

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#issuecomment-2076849139, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23D5ZGPRVLI6EFYI5VTY7DKDPAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZWHA2DSMJTHE . You are receiving this because you authored the thread.Message ID: @.***>

skycastlelily commented 3 weeks ago

Updated^^