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

Fix multiple UnitTests #5702

Merged
merged 15 commits into from Feb 27, 2022
Merged

Fix multiple UnitTests #5702

merged 15 commits into from Feb 27, 2022

Conversation

dyfer
Copy link
Member

@dyfer dyfer commented Jan 19, 2022

Purpose and Motivation

This PR fixes a number of issues with UnitTests, particularly for running them in the CI:

  • most (all?) tests using a server now use a unique server instance
    • the use of default server, particularly if it was not shut down, has led to multiple tests failing, some without reporting failure in the CI (i.e. with the Did not complete message)
    • TestFunction -test_plot arguably was hiding the server booting most covertly...
  • a number of tests was fixed
  • the link to the API quark, used by QPM for testing was updated to point to the address that's specified in the quarks directory
    • I'm not sure if this was causing any of the issues we experience in qpm or not...
  • we now have 3483 3507 passing tests with no tests that did not finish
  • I've made some changes to qpm and pointed our CI at its new branch, most notably increasing overall test suite timeout from 10 to 20 minutes (macOS tests slightly exceed 10 minutes right now)

Types of changes

  • Bug fix

To-do list

  • Code is tested
  • All tests are passing
  • Updated documentation
  • This PR is ready for review

@dyfer dyfer added the comp: testing UnitTest class, refactors of existing tests, etc.; don't use if just adding tests as part of a PR label Jan 19, 2022
@dyfer dyfer mentioned this pull request Jan 19, 2022
16 tasks
@dyfer dyfer marked this pull request as ready for review January 19, 2022 10:05
@dyfer dyfer marked this pull request as draft January 19, 2022 19:06
@dyfer dyfer marked this pull request as ready for review January 19, 2022 20:49
@dyfer dyfer marked this pull request as draft January 19, 2022 21:24
@dyfer dyfer marked this pull request as ready for review January 19, 2022 23:32
@dyfer
Copy link
Member Author

dyfer commented Jan 19, 2022

This PR is now ready for review :)

@dyfer dyfer marked this pull request as draft February 7, 2022 20:07
@dyfer
Copy link
Member Author

dyfer commented Feb 7, 2022

TestServer_boot and TestServer_clientID* tests were effectively fixed in #5715, so I re-enabled them here.
Ready for further review :)

@dyfer dyfer marked this pull request as ready for review February 7, 2022 20:56
@dyfer dyfer mentioned this pull request Feb 7, 2022
20 tasks
@joshpar joshpar self-assigned this Feb 20, 2022
Copy link
Member

@joshpar joshpar left a comment

Choose a reason for hiding this comment

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

Basically approved but want to consider the crucialfelix bit... otherwise, fine from my side to merge in

.github/workflows/actions.yml Show resolved Hide resolved
}
test_controlFree {
var s,busses;
s = Server.default;
s = Server(thisMethod.name);
Copy link
Member

Choose a reason for hiding this comment

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

I like this!

@joshpar joshpar merged commit 261b42f into supercollider:develop Feb 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: testing UnitTest class, refactors of existing tests, etc.; don't use if just adding tests as part of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants