[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 07/11] iotests: add findtests.py

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v7 07/11] iotests: add findtests.py
Date: Fri, 22 Jan 2021 16:52:20 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1

22.01.2021 16:34, Kevin Wolf wrote:
Am 22.01.2021 um 14:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
22.01.2021 15:45, Kevin Wolf wrote:
Am 22.01.2021 um 12:57 hat Vladimir Sementsov-Ogievskiy geschrieben:
22.01.2021 14:48, Kevin Wolf wrote:
Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
+    def add_group_file(self, fname: str) -> None:
+        with open(fname) as f:
+            for line in f:
+                line = line.strip()
+                if (not line) or line[0] == '#':
+                    continue
+                words = line.split()
+                test_file = self.parse_test_name(words[0])
+                groups = words[1:]

The previous version still had this:

+                if test_file not in self.all_tests:
+                    print(f'Warning: {fname}: "{test_file}" test is not found.'
+                          ' Skip.')
+                    continue

Why did you remove it? I found this useful when I had a wrong test name
in my group.local. Now it's silently ignored.

Because now we use parse_test_name which will raise ValueError, so we
will not go to this if anyway.

So, wrong name will not be silently ignored, check will fail, and
you'll have to fix group file.. It is suitable?

It doesn't, though.

Oh, wait... Is it possible that you lost support for group.local
altogether? grep for "group.local" comes up empty, and add_group_file()
is only defined, but never called.

So the reason for the behaviour seems to be that it doesn't even try to
parse the group file.

Ooops, you are right :( I've dropped an extra layer of indirection to
make things simpler and group.local was lost. It's the reason to send
v8, I'll do it now.

You can wait with sending v8 until I've completed review in case
something else comes up. So far I'm done with the changes to the part
that I reviewed last time and apart from this bug they look good to me.
Now it's the remaining patches.

OK. This thing is to be fixed in [10], not here, I'll send a squash-in

In a mean time, reverting 06 for now is OK for me.

Not a big deal if we get it fixed soon, it only becomes a problem if the
rest of this series gets shelved for a longer time. Maybe we can
complete it today, maybe on Monday, and then I'll send a pull request
right away.


Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]