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

Python unit tests are non-deterministic #1635

Closed
chipkent opened this issue Dec 4, 2021 · 5 comments
Closed

Python unit tests are non-deterministic #1635

chipkent opened this issue Dec 4, 2021 · 5 comments
Assignees
Milestone

Comments

@chipkent
Copy link
Member

chipkent commented Dec 4, 2021

Python unit tests use sleeps, which make them non-deterministic. For example:

    def test_snapshot_timetable(self):
        t = self.session.time_table(period=10000000)
        time.sleep(1)
        pa_table = t.snapshot()
        self.assertGreaterEqual(pa_table.num_rows, 1)
@jmao-denver
Copy link
Contributor

had a discussion with @niloc132, he pointed out that the default refreshing period for time tables is 1s. So this test case definitely is not deterministic. One fix is to change the sleep time to 2s to make sure the time table refreshes at least once. @chipkent let me know what you think.

@chipkent
Copy link
Member Author

chipkent commented Dec 9, 2021

Better to get @rcaudy 's thoughts.

@rcaudy
Copy link
Member

rcaudy commented Dec 10, 2021

My thoughts haven't changed since the last time we discussed something like this. Don't rely on sleeps to guarantee asynchronous behavior in unit tests.
There are exactly two ways forward for this test:

  1. Use the existing UGP unit test support to control the refresh behavior, and thereby ensure that the TimeTable refreshes after the desired time has elapsed.
  2. Delete this test. It seems to be trying to test the correct behavior of TimeTable, which is better done from Java on the engine side. You can test an initial value if you want, but it's not clear that this is a meaningful unit test for the Python wrapper.

@rcaudy
Copy link
Member

rcaudy commented Dec 10, 2021

Revising my comment. I thought this was a test for the server-side wrappers. On the client you don't have the option to control the server's UGP. The test should just be deleted, or revised to not expect data to tick.

@jmao-denver
Copy link
Contributor

jmao-denver commented Dec 17, 2021

fixed by PR #1679

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants