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

Unit testing of thread scheduling API + additional fixes #416

Merged
merged 10 commits into from
Apr 30, 2024

Conversation

erlingrj
Copy link
Collaborator

@erlingrj erlingrj commented Apr 28, 2024

  • Add unit test of the scheduling API
  • Fix a few more issues in the scheduling API
  • Improve unit test CMake system
  • Move RTI unit tests to the RTI folder and reuse the original CMake script
  • Disable unit tests on Windows. They haven't been running anyways.

@erlingrj erlingrj added the enhancement Enhancement of existing feature label Apr 28, 2024
@erlingrj erlingrj requested a review from lhstrh April 28, 2024 17:21
@erlingrj erlingrj marked this pull request as draft April 28, 2024 20:06
@erlingrj erlingrj force-pushed the thread-sched-test-and-fix branch from 4d34d7d to 7b04b8a Compare April 29, 2024 08:01
@erlingrj erlingrj force-pushed the thread-sched-test-and-fix branch from 7b04b8a to 0fe300f Compare April 29, 2024 08:11
@erlingrj erlingrj marked this pull request as ready for review April 29, 2024 08:22
@erlingrj
Copy link
Collaborator Author

OK, @lhstrh this is finally ready for review

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

I reviewed this earlier but forgot to submit it 🤦

.github/workflows/unit-tests.yml Outdated Show resolved Hide resolved
trace/impl/lib/lf-trace-impl.a Outdated Show resolved Hide resolved
test/scheduling/scheduling_api_test.c Outdated Show resolved Hide resolved
@erlingrj
Copy link
Collaborator Author

@lhstrh the comments should all be addressed now

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

A lot of CMake magic here that I have a hard time following, but this generally looks good!

CMakeLists.txt Show resolved Hide resolved
core/federated/RTI/test/rti_common_test.c Outdated Show resolved Hide resolved
@erlingrj
Copy link
Collaborator Author

We appear to have a problem with EnclaveFederatedRequestStop, this test is a little strange since it uses enclaves which arent officially supported yet. And the initial implementation does not work with federations. It does, however, compile and run correctly on my machine. But we might want to remove that test

@lhstrh lhstrh enabled auto-merge April 29, 2024 17:22
@lhstrh lhstrh changed the title Add unit testing of thread scheduling API + additional fixes Unit testing of thread scheduling API + additional fixes Apr 29, 2024
@lhstrh
Copy link
Member

lhstrh commented Apr 30, 2024

It looks like unit tests were disabled for Windows, but we still expect them to pass in order to be pushed onto the merge queue...

@erlingrj erlingrj force-pushed the thread-sched-test-and-fix branch from b6afbaf to a1e043b Compare April 30, 2024 10:53
@lhstrh lhstrh added this pull request to the merge queue Apr 30, 2024
Merged via the queue into main with commit 329528f Apr 30, 2024
53 of 56 checks passed
@erlingrj erlingrj deleted the thread-sched-test-and-fix branch May 6, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants