Out of curiosity, now I know there are methods to request longer time out. One is the method used here to request a longer time out within the test JS code. The other method is setting a timeout multiplication factor in corresponding xpcshell.ini file... Oh wait, this is a mochitest test. So for a test executed as part of mochitest, requesting longer timeout from within the JS code is the only method to request a longer time out due to some execution idiosyncrasies such as CCOV, isn't it? I am asking because I have tried to cope with the need of longer timeout for valgrind- un (much slower even than CCOV run. Usually close to x10 or more slower than the normal run.) by changing the timeout from runtests.py. But it has not worked well although it worked rather well during |make mozmill| test days. Yes, I did change the timeout by changing some test code files back then. So if the use of "2" as in requestLongerTimeout(AppConstants.MOZ_CODE_COVERAGE ? 2 : 1); good for CCOV run, what should I use for |AppConstants.MOZ_CODE_COVERAGE| to cope with Valgrind run. Hmm... https://searchfox.org/comm-central/source/mozilla/remote/cdp/test/browser/head.js#25 shows ``` function getTimeoutMultiplier() { if ( AppConstants.DEBUG || AppConstants.MOZ_CODE_COVERAGE || AppConstants.ASAN || AppConstants.TSAN ) { return 4; } return 1; } ``` It seems that valgrind run is not considered at all. :-( valgrind has helped me to find memory-related issues over the years. It is a pity it is not addressed very well. I wonder how firefox valgrind test run (I read that is done regularly) is handling the timeout issues. Now I realize that there are places where AppConstans.MOZ_CODE_COVERAGE is referenced. https://searchfox.org/comm-central/search?q=AppConstants.MOZ_CODE_COVERAGE&path=&case=false®exp=false MOZ_CODE_COVERAGE is defined here. https://searchfox.org/comm-central/source/mozilla/toolkit/modules/AppConstants.sys.mjs#387 Maybe I should define MOZ_VALGRIND_RUN or something like that here, and see if the modification of getTimeoutMultiplier as follows will help. ``` function getTimeoutMultiplier() { if (AppConstants.MOZ_VALGRIND_RUN) { return 20; } if ( AppConstants.DEBUG || AppConstants.MOZ_CODE_COVERAGE || AppConstants.ASAN || AppConstants.TSAN ) { return 4; } return 1; } ``` Wait, the patch proposed here takes care of CCOV run, but it does take into account ASAN or TSAN run, so maybe we should instead use this |getTimeoutMultiplier()| instead? (I have sometimes run xpcshell/mochitest in the past using ASAN-enabled binaries). I am not sure how the files are included, etc. and so the scope of the function is not very clear to me. So I cannot propose how to call the function from each test. In any case, looking at this patch makes me to try to proceed with valgrind timeout issue from the modification of each test, rather than from the modification of runtests.py, and maybe a change of single function |getTimeoutMultiplier()| may go a long way. (And I still wonder how Firefox manages longer time out under valgrind, if the valgrind test is executed regularly.)
Bug 1826010 Comment 8 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Out of curiosity, now I know there are methods to request longer time out. One is the method used here to request a longer time out from within the test JS code. The other method is setting a timeout multiplication factor in corresponding xpcshell.ini file... Oh wait, this is a mochitest test. So for a test executed as part of mochitest, requesting longer timeout from within the JS code is the only method to request a longer time out due to some execution idiosyncrasies such as CCOV, isn't it? I am asking because I have tried to cope with the need of longer timeout for valgrind-run (much slower even than CCOV run. Usually close to x10 or more slower than the normal run.) by changing the timeout from runtests.py. But it has not worked well although it worked rather well during |make mozmill| test days. Yes, I did change the timeout by changing some test code files back then. So if the use of "2" as in requestLongerTimeout(AppConstants.MOZ_CODE_COVERAGE ? 2 : 1); good for CCOV run, what should I use for |AppConstants.MOZ_CODE_COVERAGE| to cope with Valgrind run. Hmm... https://searchfox.org/comm-central/source/mozilla/remote/cdp/test/browser/head.js#25 shows ``` function getTimeoutMultiplier() { if ( AppConstants.DEBUG || AppConstants.MOZ_CODE_COVERAGE || AppConstants.ASAN || AppConstants.TSAN ) { return 4; } return 1; } ``` It seems that valgrind run is not considered at all. :-( valgrind has helped me to find memory-related issues over the years. It is a pity it is not addressed very well. I wonder how firefox valgrind test run (I read that is done regularly) is handling the timeout issues. Now I realize that there are places where AppConstans.MOZ_CODE_COVERAGE is referenced. https://searchfox.org/comm-central/search?q=AppConstants.MOZ_CODE_COVERAGE&path=&case=false®exp=false MOZ_CODE_COVERAGE is defined here. https://searchfox.org/comm-central/source/mozilla/toolkit/modules/AppConstants.sys.mjs#387 Maybe I should define MOZ_VALGRIND_RUN or something like that here, and see if the modification of getTimeoutMultiplier as follows will help. ``` function getTimeoutMultiplier() { if (AppConstants.MOZ_VALGRIND_RUN) { return 20; // <----- large factor for valgruind run } if ( AppConstants.DEBUG || AppConstants.MOZ_CODE_COVERAGE || AppConstants.ASAN || AppConstants.TSAN ) { return 4; } return 1; } ``` Wait, the patch proposed here takes care of CCOV run, but it does NOT take into account ASAN or TSAN run, so maybe we should instead use this |getTimeoutMultiplier()| instead? (I have sometimes run xpcshell/mochitest in the past using ASAN-enabled binaries). I am not sure how the files are included, etc. and so the scope of the function is not very clear to me. So I cannot propose how to call the function, |getTimeoutMultipler()|, from each test. https://searchfox.org/comm-central/source/mozilla/remote/cdp/test/browser/head.js#22 defines const TIMEOUT_MULTIPLIER = getTimeoutMultiplier(); So it has the same scope issue, but we may want to include |TIMEOUT_MULTIPLER| instead. In any case, looking at this patch makes me to try to proceed with valgrind timeout issue from the modification of each test, rather than from the modification of runtests.py, and I suspect (maybe too optimistically) a change of single function |getTimeoutMultiplier()| may go a long way. (And I still wonder how Firefox manages longer time out under valgrind, if the valgrind test is executed regularly.) Edit: some typos were fixed. References to |TIMOUT_MULTIPLER| added.