Remove LTP sudo, Change sudo as parameter, Add full test support for LTP #3775
Conversation
microsoft/testsuites/ltp/ltp.py
Outdated
| @@ -72,11 +72,12 @@ def dependencies(self) -> List[Type[Tool]]: | |||
| def can_install(self) -> bool: | |||
| return True | |||
|
|
|||
| def __init__(self, node: Node, source_file: str, *args: Any, **kwargs: Any) -> None: | |||
| super().__init__(node, args, kwargs) | |||
| def __init__(self, node: Node, *args: Any, **kwargs: Any) -> None: | |||
There was a problem hiding this comment.
Give the value with a default value, instead of using kwargs.get
There was a problem hiding this comment.
Do you mean add the following lines in init()?
self._source_file = ""
self._run_as_sudo = True
There was a problem hiding this comment.
it should be like def __init__(self, node: Node, source_file: str = "", run_as_sudo:bool =True, *args: Any, **kwargs: Any) -> None:
microsoft/testsuites/ltp/ltp.py
Outdated
| git_tag = kwargs.get("git_tag", "") | ||
| self._git_tag = git_tag if git_tag else self.DEFAULT_LTP_TESTS_GIT_TAG | ||
| self._source_file = source_file | ||
| self._source_file = kwargs.get("source_file", "") | ||
| self._run_as_sudo = kwargs.get("sudo", True) |
There was a problem hiding this comment.
A similar question like Kselftest. Any difference to run with/out sudo?
There was a problem hiding this comment.
For using released file.
Kselftest: don't have config, can direct unzip the release file and run.
LTP: does have config step, this step involve lots of sudo issue.
There was a problem hiding this comment.
I mean for LTP, if run without sudo, does it work?
microsoft/testsuites/ltp/ltpsuite.py
Outdated
| @@ -71,6 +71,9 @@ def verify_ltp_lite( | |||
| # If not provided, we will find a disk with enough space | |||
| block_device = variables.get("ltp_block_device", None) | |||
|
|
|||
| run_full_ltp_test = variables.get("run_full_ltp_test", False) | |||
There was a problem hiding this comment.
Is there any requirement to run all of them? This flag is easy to be conflict with other parameters. Please create another test case called verify_ltp. The only difference is the new test case is to run all, if no list given.
There was a problem hiding this comment.
Is there any requirement to run all of them?
[panfeng xue]: yes. it's required from Cooperate team
I am fine with 2 paths: Create another verify_ltp_full() or just using the same test suite verify_ltp_lite.
From my view, their changes are very limited, only one line code change. Depends how you think this question
There was a problem hiding this comment.
I don't want to add a parameter called run_full_ltp_test, it conflicts with existing parameters like ltp_test and ltp_skip_test
paxue
force-pushed
the
paxue/linkedin3_pr
branch
from
b5d6a9b to
ef9e76c
Compare
change sudo as parameter fix the path issue for ltp test using uploaded file
paxue
force-pushed
the
paxue/linkedin3_pr
branch
from
ef9e76c to
ded131d
Compare
Add full test support for LTP
Remove LTP sudo, change as parameter