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

Feature/add test timeout #181

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Navid-Mashayekh
Copy link

@Navid-Mashayekh Navid-Mashayekh commented Feb 13, 2022

New function "TIMEOUT_FOR_EACH_TEST" added to control time out for each test.
This implements the functionality for #142.

@sagatowski
Copy link
Member

I'll start reviewing this PR as soon as this PR is completed/merged. Once it's merged, you will most likely need to merge from trunk again as the changes in the PR #159 will most likely break your branch.

I realized that there might be quite a few of these annotations the more functions we want to add to TcUnit. Is there any way we could achieve annotations in the same way as JUnit?

What does this PR do compared to PR #179? What do you want to happen if you mix AWAIT_ASSERTIONS and TEST_TIMEOUT?

What happens if a timeout happens?

Could you also link this to an issue (that describes what problem this solves or what feature is added? As it is right now I have to read the code to maybe understand what problem this solves).

@sagatowski
Copy link
Member

I just relized that this is connected to #142.

@sagatowski
Copy link
Member

@Navid-Mashayekh How does this differ from #179? When would one want to use one over the other? Aren't they redundant?

The standard procedure is to simply use a timer or a state-machine to control the timeout for a test.

@Navid-Mashayekh
Copy link
Author

The main idea behind the timeout is to be able to quit the test in a pipeline when it stucks in a test (eq: missing TEST_FINISHED, etc) otherwise the job will keep the ci/cd busy for hours!
#179 is about delaying the assertions, because unlike pc software many plc logics are based on cyclic processing. In these cases as you said we should use a timer in the test suit or a state machine. I think it might be useful to have this feature integrated, but having both (timeout and delay) is also tricky! since they are in contrast they can break tests unintentionaly!

@sagatowski
Copy link
Member

I see!
This is redundant to the timeout flag in the TcUnit-Runner.

-u [OPTIONAL] Timeout the process(es) with an error after X minutes

The build job will anyway continue to run unless there is an actual return from the process of which it invoked.

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