Re: CVS commit: src/tests/util/sh
On 11/14/11 3:23 PM, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Mon Nov 14 20:23:29 UTC 2011 Modified Files: src/tests/util/sh: Makefile Added Files: src/tests/util/sh: t_evaltested.sh Log Message: Add a test for PR/45613 (eval failing in a tested context) This looks like a poor name for the program: in general, if the test program contains a single test case and both have the same name, that's a bad symptom. Following prior art in this same directory, the program should have probably been named t_eval and the single test that it currently has tested_context. This would leave room for the future addition of eval-related tests, whereas the current naming does not.
Re: CVS commit: src/external/bsd/atf/dist
On 11/16/11 12:46 PM, Christos Zoulas wrote:
Module Name:src
Committed By: christos
Date: Wed Nov 16 17:46:16 UTC 2011
Modified Files:
src/external/bsd/atf/dist/atf-c++/detail: text.cpp text.hpp
src/external/bsd/atf/dist/atf-run: requirements.cpp test-program.cpp
Log Message:
PR/45619: jmmv: Allow atf tests to request a minimum amount of memory
Could you run intrusive changes to atf for review please?
The current changes are incomplete and inappropriate, so they are going
to make integration upstream a pain.
+int64_t
+impl::to_number(const std::string str)
+{
+ int64_t num;
+ ::dehumanize_number(str.c_str(),num);
This adds a dependency on NetBSD that should not be there.
[...]
+static
+std::string
+check_memory(const std::string memory)
+{
+// Make sure we have enough memory
+int64_t memneed = atf::text::to_number(memory);
+int64_t memavail;
+size_t len = sizeof(memavail);
+
+if (::sysctlbyname(hw.usermem64,memavail,len, NULL, 0) == -1) {
Same here and below in the same function. This is NetBSD specific and
there is no provisions to support other systems.
[...]
Lastly, this has been added without absolutely no tests and no
documentation. (And there seem to be whitespace issues, although I
can't tell if these are my mua's fault...)
Re: CVS commit: src/tests/lib/libc/regex
On Nov 16, 10:23am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/tests/lib/libc/regex | Since this test is succeeding on amd64 with only 128MB allocated in | qemu, do we really need to set the minimum to 500? Ok, I will reduce it. christos
Re: CVS commit: src/external/bsd/atf/dist
In article 4ec3f8f7.8020...@netbsd.org, Julio Merino j...@netbsd.org wrote: On 11/16/11 12:46 PM, Christos Zoulas wrote: Module Name: src Committed By:christos Date:Wed Nov 16 17:46:16 UTC 2011 Modified Files: src/external/bsd/atf/dist/atf-c++/detail: text.cpp text.hpp src/external/bsd/atf/dist/atf-run: requirements.cpp test-program.cpp Log Message: PR/45619: jmmv: Allow atf tests to request a minimum amount of memory Could you run intrusive changes to atf for review please? The current changes are incomplete and inappropriate, so they are going to make integration upstream a pain. Well, there is really no portable way to find the total available memory of the system that I know of and I did not want to add ifdefs or machinery to get this working on other OS's. If you know otherwise, feel free to fix it. The changes are really small and they are additions only. I fail to see how it is going to be difficult to integrate. The only difficulty here is to make them portable across OS's, and that can be done with a getrealmemory() stub function per OS, and include the sources for humanize_number for the ones that don't have it. christos
Re: CVS commit: src/external/bsd/atf/dist
On 11/16/11 1:51 PM, Christos Zoulas wrote: In article4ec3f8f7.8020...@netbsd.org, Julio Merinoj...@netbsd.org wrote: On 11/16/11 12:46 PM, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Wed Nov 16 17:46:16 UTC 2011 Modified Files: src/external/bsd/atf/dist/atf-c++/detail: text.cpp text.hpp src/external/bsd/atf/dist/atf-run: requirements.cpp test-program.cpp Log Message: PR/45619: jmmv: Allow atf tests to request a minimum amount of memory Could you run intrusive changes to atf for review please? The current changes are incomplete and inappropriate, so they are going to make integration upstream a pain. Well, there is really no portable way to find the total available memory of the system that I know of and I did not want to add ifdefs or machinery to get this working on other OS's. If you know otherwise, feel free to fix it. I know there is no portable way, but at least we can default to do nothing if this is not supported. It's better than not building :-P The changes are really small and they are additions only. I fail to see how it is going to be difficult to integrate. Yes, they are small because this is just one change. But more may come. I'll then have to go and rewrite all these local changes with portability in mind. When the time to integrate a new release comes, I'll have to mess around with lots merge conflicts, because the upstream code will look nothing like what we have (hence why I asked for this to be reviewed first). Of course, if we assume I keep good track of all local changes and integrate them upstream (I do try), I could ignore the local changes altogether during the conflicts resolution and use the upstream copies... but that's... dangerous because I can miss some little thing. Specially if the local changes are made without tests, because then it'll be impossible for me to spot when such changes are not preserved.
Re: CVS commit: src/external/bsd/atf/dist
In article 4ec40d98.4070...@netbsd.org, Julio Merino j...@netbsd.org wrote: I know there is no portable way, but at least we can default to do nothing if this is not supported. It's better than not building :-P Oh, I can arrange that. #ifdef __NetBSD__ :-) But in my view this is worse... Yes, they are small because this is just one change. But more may come. I'll then have to go and rewrite all these local changes with portability in mind. When the time to integrate a new release comes, I'll have to mess around with lots merge conflicts, because the upstream code will look nothing like what we have (hence why I asked for this to be reviewed first). Well, this is the world we live in. Next time be the one to upgrade gdb or binutils or ssh and have to deal with 10-50K lines of diffs. You will not be complaining about a 100 line conflict after that :-) Of course, if we assume I keep good track of all local changes and integrate them upstream (I do try), I could ignore the local changes altogether during the conflicts resolution and use the upstream copies... but that's... dangerous because I can miss some little thing. Specially if the local changes are made without tests, because then it'll be impossible for me to spot when such changes are not preserved. I understand, and when I get more time I will write tests. I just wanted to stop our tests from failing in a non-hacky way quickly and I have achieved my goal with less than 30 minutes of coding. christos
Re: CVS commit: src/external/bsd/atf/dist
On 11/16/11 3:11 PM, Christos Zoulas wrote: In article4ec40d98.4070...@netbsd.org, Julio Merinoj...@netbsd.org wrote: I know there is no portable way, but at least we can default to do nothing if this is not supported. It's better than not building :-P Oh, I can arrange that. #ifdef __NetBSD__ :-) But in my view this is worse... Yeah, leave it as is for now. I'll have to do something similar anyway though, but if I can implement the code for the most common systems, that's probably good enough. Yes, they are small because this is just one change. But more may come. I'll then have to go and rewrite all these local changes with portability in mind. When the time to integrate a new release comes, I'll have to mess around with lots merge conflicts, because the upstream code will look nothing like what we have (hence why I asked for this to be reviewed first). Well, this is the world we live in. Next time be the one to upgrade gdb or binutils or ssh and have to deal with 10-50K lines of diffs. You will not be complaining about a 100 line conflict after that :-) That's true... but do we have a chance of getting things integrated into gdb quickly and/or on our own to prevent those 10-50K diffs? For atf we do ;-) (OK, I can definitely try to improve on the quickly side...) Anyway, just something to keep in mind. Still, I'd appreciate running changes for review at least. I'm open to overlooking details like these that don't fit upstream if that's going to make things easier temporarily.
