Re: RFR: 8275170: Some jtreg sound tests should be marked headful
On Sun, 24 Oct 2021 16:45:15 GMT, Phil Race wrote: > This makes the tests eligible to be run on headful systems. In practice it > seems like ONLY headful systems actually have the sound devices. The sound > keyword isn't understood by anything yet, so adding headful is the only way > we have right now of ensuring these tests are eligible to be run on systems > that have the necessary support. This is not the right approach, it was made a hard work to eliminate the headful keyword from any tests including sound which are not throw the headless exception or something. The "headful" means that the test will fail otherwise due to UI session missing. We should not abuse it to select some other unrelated tests. Like we do not doing this for printer. The problem you try to solve is that you have a "bad" system where nothing is configured and the "good" system where UI/sound(what about printer?) are configured. And you want to exclude all tests from the "bad" host and run them on the "good" one by using the headful keyword which is not targeted for this usecase. > As I explained in the initial PR description headful is also useful for > implying exclusive access. Such exclusive access should not be needed by the sound, in fact it was found a few hard too spot concurrency bugs when we start to run the sound tests in parallel. > And without this keyword it means that these tests are ALWAYS run on systems > without sound device support. Is it always just because the sound is not configured on the systems where you run the tests without headful keyword? How it is related to the "headfulness" of the systems? > So for years we've effectively problem listed these tests due to the tests > not having headful and silently passing when there's no sound. Without > headful no one runs these on systems that have the needed device. This was done intentionally that everybody who run the tests also run the tests for the sound, moreover it was included the openjdk tier3, before it was accidentally removed from there. > The 2 keywords give flexibility - anyone who wants to filter when its marked > headful and needing sound can do this, but just adding sound right now will > solve nothing. Such flexibility exists already, it is possible to run test/jdk/javax/sound on any computer you want and exclude it from the jdk_desktop test run. > And there are a couple of tests NOT in OpenJDK that already do this so we > have precedent - and in a previous life one was added by you and the other > approved by you .. If it is not in the OpenJDK then we do not need to discuss it here and that's not actually a precedent. We should investigate each case one by one, the headful keyword does not solve any issue on windows, since all windows are headful(or most of them where we interested in the audio), on macOS the sound subsystem is built-in. And on Linux it is possible to configure sound as well and it will work fine w/o X server. I think most of the sound tests are written according to the spec which does not required UI, and if it was necessary to make it pass by running on the system which have UI system configured make me think that some product bug was workaround. - PR: https://git.openjdk.java.net/jdk/pull/6086
Re: RFR: 8275170: Some jtreg sound tests should be marked headful
On Fri, 22 Oct 2021 19:01:27 GMT, Phil Race wrote: > This fix adds "headful sound" keywords to a number of javax/sound jtreg tests > > The jtreg javax.sound tests are written in such a way that if a needed audio > device > or other platform resource is not available, they just exit with a "pass" for > the test. > It is as if headful tests just swallowed HeadlessException and issued a pass. > If we allowed that we'd be effectively testing almost nothing of real > importance. > > Currently almost all sound tests have no keyword like "headful" to indicate > they > may need access to a hardware resource to do anything useful so a similar > situation arises there except when someone runs those tests manually on > a local system which has audio devices. > > Of course "headful" and "audio device" aren't exactly interchangeable terms > but if tests are allowed to be run - or worse usually or always run - in a > situation > where they potentially are on a system without an audio device (a headless > VM) or are running in > a session which doesn't have full desktop access - which may in some > cases be how audio device access is granted, then they are more likely > to be running in this scenario where the testing isn't valid. > > Another point is that headful tests must be run one at a time - no > concurrency because > you can't have multiple tests moving the mouse at the same time > Whereas for headless tests, a test harness may choose to run concurrently - > perhaps even in the same VM > When tests that really need access to an audio device are run it is probably > safer to consider > the headful attribute as implying one test at a time is best .. granted on a > desktop it isn't > usual for a single app to be able to monopolize access to a speaker, but for > input devices > that is what you'd generally expect. > > By no means am I sure that the tests being updated here are the full set that > need tagging. > There are a lot of tests and they are all known to pass on systems with NO > audio > devices, so the search has been for tests which call APIs which will > definitely > need access to some device in order to do anything useful. > So likely there are more to be added - either by a reviewer pointing them out > or by later discovery. > Alternatively we could mark ALL sound tests headful .. but given where we are > starting from > that might be erring unnecessarily far in the opposite direction. > > By adding both headful and sound keywords it gives options to someone who > wants to identify the tests and perhaps run just tests which need "sound" on > some > config they know supports audio but isn't headful. This makes the tests eligible to be run on headful systems. In practice it seems like ONLY headful systems actually have the sound devices. The sound keyword isn't understood by anything yet, so adding headful is the only way we have right now of ensuring these tests are eligible to be run on systems that have the necessary support. As I explained in the initial PR description headful is also useful for implying exclusive access. And without this keyword it means that these tests are ALWAYS run on systems without sound device support. So for years we've effectively problem listed these tests due to the tests not having headful and silently passing when there's no sound. Without headful no one runs these on systems that have the needed device. So this solves a big hole that we aren't testing this case. The 2 keywords give flexibility - anyone who wants to filter when its marked headful and needing sound can do this, but just adding sound right now will solve nothing. And there are a couple of tests NOT in OpenJDK that already do this so we have precedent - and in a previous life one was added by you and the other approved by you .. - PR: https://git.openjdk.java.net/jdk/pull/6086
Re: RFR: 8275170: Some jtreg sound tests should be marked headful
On Fri, 22 Oct 2021 19:01:27 GMT, Phil Race wrote: > This fix adds "headful sound" keywords to a number of javax/sound jtreg tests > > The jtreg javax.sound tests are written in such a way that if a needed audio > device > or other platform resource is not available, they just exit with a "pass" for > the test. > It is as if headful tests just swallowed HeadlessException and issued a pass. > If we allowed that we'd be effectively testing almost nothing of real > importance. > > Currently almost all sound tests have no keyword like "headful" to indicate > they > may need access to a hardware resource to do anything useful so a similar > situation arises there except when someone runs those tests manually on > a local system which has audio devices. > > Of course "headful" and "audio device" aren't exactly interchangeable terms > but if tests are allowed to be run - or worse usually or always run - in a > situation > where they potentially are on a system without an audio device (a headless > VM) or are running in > a session which doesn't have full desktop access - which may in some > cases be how audio device access is granted, then they are more likely > to be running in this scenario where the testing isn't valid. > > Another point is that headful tests must be run one at a time - no > concurrency because > you can't have multiple tests moving the mouse at the same time > Whereas for headless tests, a test harness may choose to run concurrently - > perhaps even in the same VM > When tests that really need access to an audio device are run it is probably > safer to consider > the headful attribute as implying one test at a time is best .. granted on a > desktop it isn't > usual for a single app to be able to monopolize access to a speaker, but for > input devices > that is what you'd generally expect. > > By no means am I sure that the tests being updated here are the full set that > need tagging. > There are a lot of tests and they are all known to pass on systems with NO > audio > devices, so the search has been for tests which call APIs which will > definitely > need access to some device in order to do anything useful. > So likely there are more to be added - either by a reviewer pointing them out > or by later discovery. > Alternatively we could mark ALL sound tests headful .. but given where we are > starting from > that might be erring unnecessarily far in the opposite direction. > > By adding both headful and sound keywords it gives options to someone who > wants to identify the tests and perhaps run just tests which need "sound" on > some > config they know supports audio but isn't headful. I am not sure this is the right thing to do, as you pointed out the headless system may or may not have the sound configured, similar to the headful system may not have a sound. I see a lot of headful systems in the cloud where the audio device is not configured, and the opposite where the headless system actually has some audio devices. And this change just makes things complicated by assignee the headful keyword to the unrelated sound system. Actually, it is the step in the opposite direction that was done when these tests were moved to tier3, at that the headful keyword was removed from these tests to make tier3 coverage as much as possible. I am still not sure what problem do you want to solve? If the problem is to run the tests on the system where the sound is configured then just run them there, so it will not be necessary to validate each test does it really need a sound/headful keyword or not, otherwise what is the benefit? - PR: https://git.openjdk.java.net/jdk/pull/6086
RFR: 8275170: Some jtreg sound tests should be marked headful
This fix adds "headful sound" keywords to a number of javax/sound jtreg tests The jtreg javax.sound tests are written in such a way that if a needed audio device or other platform resource is not available, they just exit with a "pass" for the test. It is as if headful tests just swallowed HeadlessException and issued a pass. If we allowed that we'd be effectively testing almost nothing of real importance. Currently almost all sound tests have no keyword like "headful" to indicate they may need access to a hardware resource to do anything useful so a similar situation arises there except when someone runs those tests manually on a local system which has audio devices. Of course "headful" and "audio device" aren't exactly interchangeable terms but if tests are allowed to be run - or worse usually or always run - in a situation where they potentially are on a system without an audio device (a headless VM) or are running in a session which doesn't have full desktop access - which may in some cases be how audio device access is granted, then they are more likely to be running in this scenario where the testing isn't valid. Another point is that headful tests must be run one at a time - no concurrency because you can't have multiple tests moving the mouse at the same time Whereas for headless tests, a test harness may choose to run concurrently - perhaps even in the same VM When tests that really need access to an audio device are run it is probably safer to consider the headful attribute as implying one test at a time is best .. granted on a desktop it isn't usual for a single app to be able to monopolize access to a speaker, but for input devices that is what you'd generally expect. By no means am I sure that the tests being updated here are the full set that need tagging. There are a lot of tests and they are all known to pass on systems with NO audio devices, so the search has been for tests which call APIs which will definitely need access to some device in order to do anything useful. So likely there are more to be added - either by a reviewer pointing them out or by later discovery. Alternatively we could mark ALL sound tests headful .. but given where we are starting from that might be erring unnecessarily far in the opposite direction. By adding both headful and sound keywords it gives options to someone who wants to identify the tests and perhaps run just tests which need "sound" on some config they know supports audio but isn't headful. - Commit messages: - 8275170: Some jtreg sound tests should be marked headful Changes: https://git.openjdk.java.net/jdk/pull/6086/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6086=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275170 Stats: 63 lines in 58 files changed: 59 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6086.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6086/head:pull/6086 PR: https://git.openjdk.java.net/jdk/pull/6086
