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

add a dumb timeout logic #83

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

add a dumb timeout logic #83

wants to merge 8 commits into from

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Aug 28, 2023

issue: #42

a few notes on this implementation:

  • the timeout is hardcoded to 3 seconds for now. we can add a configuration in per-test json files when necessary.
  • unit test doesn't cover the new code. i have no plan to write tests at this point.
  • adapters may need something like wasi-testsuite-adapter.py: handle SIGTERM yamt/toywasm#117 for better cleanup

@yamt yamt changed the title add a dump timeout logic add a dumb timeout logic Aug 29, 2023
note: the end of file change is an unrelated editor artifact. (nvi2)
yamt added a commit to yamt/toywasm that referenced this pull request Aug 29, 2023
yamt added a commit to yamt/toywasm that referenced this pull request Aug 29, 2023
@yamt
Copy link
Contributor Author

yamt commented Sep 8, 2023

@loganek please review when you have time. this is not urgent for me.

@loganek
Copy link
Collaborator

loganek commented Sep 11, 2023

Thanks for the change, and sorry for being late with the review. I'll try to get to it by end of this week.

@yamt
Copy link
Contributor Author

yamt commented Jan 29, 2024

@loganek ping


On a timeout, the test runner sends SIGTERM singal to the adapter process.
When receiving the signal, the adapter process should clean up and exit
as soon as possible.
Copy link
Collaborator

@loganek loganek Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you document the current timeout?

@@ -63,5 +70,5 @@ def _validate_dict(cls: Type[T], dict_config: Dict[str, Any]) -> None:
class TestCase(NamedTuple):
name: str
config: Config
result: Result
result: Union[Result | SkippedResult | TimedoutResult]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never seen this syntax before; IIRC it should be either Union[Result, SkippedResult, TimedoutResult] or Result | SkippedResult | TimedoutResult, see https://docs.python.org/3/library/typing.html#typing.Union

test_output = runtime.run_test(test_path, config.args, config.env, config.dirs)
try:
test_output = runtime.run_test(test_path, config.args, config.env, config.dirs)
result: Union[Result | TimedoutResult] = _validate(validators, config, test_output)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

2 participants