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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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
This was referenced Jan 19, 2022
Merged
Merged
@dyfer dyfer mentioned this pull request Jan 19, 2022
Open
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 dyfer mentioned this pull request Jan 19, 2022
Open
@dyfer
Copy link
Member Author

dyfer commented Jan 19, 2022

This PR is now ready for review :)

dyfer added 13 commits February 7, 2022 11:47
@dyfer
[UnitTests] Enable some TestNodeProxy tests again
a6a3499 
test_copy_nodeMapIsCopied and test_synthDef_isReleased_afterFree
@dyfer
[UnitTests] fix TestNodeProxySeti
faf150e
@dyfer
[UnitTests] Exclude some broken tests
3eb7d37
@dyfer
[UnitTests] Exclude CondVar tests
af7350b
@dyfer
[TestIndexUGenRates] Use a dedicated server object
4046a26
@dyfer
[TestServer] Use a dedicated server object
397fdaf
@dyfer
[GHA] [UnitTests] correct API quark link
01578dc
@dyfer
[UnitTests] Fix TestBus
2fea50e 
use a dedicated server and quit it when done
@dyfer
[UnitTests] TestNetAddr: use dedicated server
e2b7587
@dyfer
[UnitTests] [TestFunction] -test_plot use a dedicated server
f30218b
@dyfer
[UnitTests] enable TestPV_ChainUGen
ab5b773
@dyfer
[UnitTests] Enable TestDoneActions test_freeSelfAndResumeNext
9c73d53
@dyfer
[GHA] [TestSuite] use a modified qpm to ignore some errors
cba0255 
This branch ignores "Persistent UI failed to open file (...)"  error
@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
Open
20 tasks
@joshpar joshpar self-assigned this Feb 20, 2022
joshpar
joshpar requested changes Feb 26, 2022
View reviewed changes
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
testsuite/classlibrary/TestBus.sc
}
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
Reviewers

@nuss nuss nuss approved these changes

@joshpar joshpar joshpar approved these changes

Assignees

@joshpar joshpar

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dyfer @joshpar @nuss