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

Consider adding "test timeout" #142

Open
RodneyRichardson opened this issue Feb 26, 2021 · 12 comments
Open

Consider adding "test timeout" #142

RodneyRichardson opened this issue Feb 26, 2021 · 12 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@RodneyRichardson
Copy link

I find I'm adding a timer (TON) to each of my tests to ensure that they don't get into an infinite loop. This is only a problem when tests run over multiple cycles (which mine do as I'm executing SFC/state machines), and the condition for calling TEST_FINISHED() is never met.

Can we have global settings (e.g. IsTimeoutEnabled : BOOL, MaxTestDuration : TIME) that will automatically fail/finish a test when it takes longer than expected?

A nice to have would be the ability to override this for a specific test (using either an attribute, or additional parameters to TEST/TEST_ORDERED).

@sagatowski
Copy link
Member

This is a good suggestion. I'll add it into the 1.2.0.0 milestone, but it might be pushed to 1.3.0.0, depending on whether I or any contributor has time to look into this. I suggest we do this as a parameter (so it's global for all tests) to maintain backwards-compability. We should also skip the IsTimeoutEnabled and just use MaxTestDuration (default it will be T#0S which means that there is no time-limit)

@sagatowski sagatowski added the enhancement New feature or request label Mar 2, 2021
@sagatowski sagatowski added this to the Release 1.2.0.0 milestone Mar 2, 2021
@Roald87
Copy link
Contributor

Roald87 commented Mar 2, 2021

Duplicate of #54?

@sagatowski
Copy link
Member

Duplicate of #54?

Yepp. Good that someone is awake. So many comments and issues now that I think I'm getting senile. I'll leave this open and close both this and #54 once this is implemented.

@RodneyRichardson
Copy link
Author

Sorry for raising a duplicate - I didn't know github had a Discussions section!

@sagatowski
Copy link
Member

No problems. You're welcome to implement!
Just don't forget to read the contribs.

@sagatowski sagatowski modified the milestones: Release 1.2.0.0, 1.3.0.0 Mar 6, 2021
@sagatowski
Copy link
Member

Moved this to release 1.3.0.0.

@sagatowski
Copy link
Member

sagatowski commented May 4, 2021

@RodneyRichardson I've assigned this issue to you. If you feel you don't have the time to look into it, just un-assign yourself.

Edit: https://github.com/tcunit/TcUnit/blob/master/CONTRIBUTING.md

@RodneyRichardson RodneyRichardson removed their assignment May 5, 2021
@RodneyRichardson
Copy link
Author

Sorry - I don't have time at the moment. This may be something that is easier to implement in v2 if you go for a different architecture (e.g. one test per FB).

@sagatowski
Copy link
Member

Anyone else care to take a look into this?

@sagatowski sagatowski added good first issue Good for newcomers help wanted Extra attention is needed labels May 5, 2021
@Navid-Mashayekh
Copy link

Hi Jakob,
I was looking into this issue and I think I can handle it but I saw that there is an open PR #159 and @stefanbesler added some cool features about time (like: duration, startTime etc.) in his code that might be useful for this issue.
So I am pending for his PR to be merged.

I am also thinking of adding Async capability to TcUnit to be able to test FBs which have delays like TONs inside of them and user expect to have an output with some delay!

@sagatowski
Copy link
Member

Hey @NoidMasha ! It's highly appreciated that you want to look into this. I've just bumped the discussion over at #159, and hopefully we'll manage to get that PR implemented as soon as possible. I agree it makes sense to wait for #159 to be merged first.

@Navid-Mashayekh
Copy link

this feature is now added in PR #181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants