Closed GoogleCodeExporter closed 8 years ago
[deleted comment]
i've withdrawn p8-697a24340740.patch, it doesn't work.
Original comment by neuhau...@sigpipe.cz
on 11 May 2010 at 11:19
Uhmm, that's a bunch of patches, can they be reconciled to a single one?
Original comment by aggraef@gmail.com
on 17 May 2010 at 4:36
sure, i submitted individual revisions on the assumption that they'd be easier
to
review piecemeal.
i'll submit the cumulated patch (i also have redone the withdrawn patch) later
today.
Original comment by neuhau...@sigpipe.cz
on 17 May 2010 at 4:40
a redone p8-697a24340740.patch is attached (run-tests-p8-d91a2a355491.patch).
issue36.patch can be recreated from the partial ones by applying them in
sequence.
Original comment by neuhau...@sigpipe.cz
on 17 May 2010 at 7:40
Attachments:
This looks mostly good to me. However, I'd prefer to have local-pure (p3) named
run-test or something like that, to make clear that it belongs to the test
system.
Also, I guess that the corresponding .in file needs to go into DISTFILES, no?
One potential cross-platform issue is the use of getopts (p7). Windows
(mingw+msys)
seems to be ok since we're running bash there. But I'm worried that this might
be a
problem with the OSX default shell, and possibly some other Un*x systems, too.
Is it
possible to get rid of this?
Original comment by aggraef@gmail.com
on 18 May 2010 at 7:22
* i'll rename local-pure
* sorry about DISTFILES, i'll be back with a fix
* getopts:
- a broad issue is that i can't tell what the supported OSs are. we'd obviously
want compatibility with as many systems as possible, but there's always a line
that
needs to be drawn. if you could just publish the set of *currently* supported
systems, i could check any changes i work on with their manuals online.
- more specifically: what's the OSX default shell? as far as i can tell, /bin/sh in
OSX is bash:
http://developer.apple.com/mac/library/documentation/Darwin/Reference/ManPages/m
an1/sh.1.html
and getopts is part of SUSv3 (POSIX), so in theory it should be quite portable
(at
least recently)
Original comment by neuhau...@sigpipe.cz
on 18 May 2010 at 12:34
Right now, we support Linux, *BSD, OSX and Windows. I don't think that anyone
has
tried AIX, HP-UX or Solaris yet, but I would think that we might want to
support at
least Solaris in the future, too.
Anyway, newer Un*x versions should probably be fine, as far as they support
POSIX.2001. The biggest issue seems to be with older OSX versions. I can't find
a
getopts manpage for OSX <= 10.3. I'm not sure whether it still makes sense to
support
that version, though, it's pretty old.
One way to proceed would be to just leave the script as it is and wait until
someone
complains that the test suite isn't working. ;-) Rewriting the script to do
without
getopts shouldn't be too hard, it parses just two short options without
arguments
after all.
Original comment by aggraef@gmail.com
on 18 May 2010 at 4:09
that's exactly how i'd deal with it
Original comment by neuhau...@sigpipe.cz
on 18 May 2010 at 4:11
Ok, if you post an updated patch then I'll give it a whirl on Linux and Windows
and
if that works, it can go into svn.
Original comment by aggraef@gmail.com
on 18 May 2010 at 4:23
here's the updated patch(es). these four are meant to be applied on top of the
patches so far:
bccd: local-pure -> run-test, per request from upstream
5806: DISTFILES += run-test.in
ff1e: fix check and recheck targets: run-tests is in $builddir
e21e: run-tests writes diff files into $builddir/test
issue36,1.patch is the total delta including these four changes.
Original comment by neuhau...@sigpipe.cz
on 19 May 2010 at 7:29
hm, and now the files...
Original comment by neuhau...@sigpipe.cz
on 19 May 2010 at 7:31
Attachments:
re the renaming of local-pure to run-test: if you ever wanted to use the
freshly
built pure for some build-time processing (pure-mode.el.in comes to mind), a
name
like local-pure would in fact be less confusing. just a thought.
Original comment by neuhau...@sigpipe.cz
on 19 May 2010 at 10:46
I don't think that run-test is very useful for running anything but the tests,
as it
sets the locale to POSIX and the verbosity level to 7 by default.
Thanks for the updated patch, I'll have a look at it asap.
Original comment by aggraef@gmail.com
on 20 May 2010 at 2:31
Committed in r3437. Thanks!
I tested it on both Linux and Windows and it appears to work fine there.
Original comment by aggraef@gmail.com
on 31 May 2010 at 4:20
Hi Roman,
I'm reopening this issue as I discovered two more quirks with the test scripts:
- make recheck doesn't run a recheck of the prelude even if there's a
prelude.diff.
- make recheck doesn't remove obsolete diff files. Maybe that's intended, but
in order to avoid confusion, I think it's better to remove the diff files for
passed checks (like make check does).
Could you please have another look at the test scripts and see whether these
issues can/should be fixed?
Original comment by aggraef@gmail.com
on 18 Jun 2010 at 8:17
Hmm, it seems that even 'make check' doesn't remove the diff files for passed
tests any more. That's definitely a bug.
Original comment by aggraef@gmail.com
on 18 Jun 2010 at 8:24
I think that the attached patch should fix issue with the sticky diffs.
Original comment by aggraef@gmail.com
on 18 Jun 2010 at 9:33
Attachments:
sorry for the delay. will look into it tonight.
Original comment by neuhau...@sigpipe.cz
on 21 Jun 2010 at 9:46
re patch from #c17:
Index: run-tests.in
===================================================================
--- run-tests.in (revision 3455)
+++ run-tests.in (working copy)
@@ -73,10 +73,11 @@
trap 'x=$?; rm -f '$tmp'; exit $x;' ALRM HUP INT TERM
if ./run-test ${pufs} < "$t" 2>&1 | ${DIFF} -u "$log" - > "$tmp"
then
- rm -f "$tmp"
+ rm -f "$tmp" "$diff"
echo "${ECHO_T}passed"
else
rc=1
+ rm -f "$diff"
mv "$tmp" "$diff"
echo "${ECHO_T}FAILED"
fi
the first hunk is good; the second one seems unnecessary ($diff is overwritten
in the next statement) and has no effect on my machine.
here's my take:
r1-8a0e877b660c.patch fixes both defects: run-tests -f removes stale diffs and
runs prelude.pure if there's a diff for that
r2-53f8c869933a.patch consolidates the two places that collected tests to run
into a single instance
issue36,2.patch combines both changes.
Original comment by neuhau...@sigpipe.cz
on 22 Jun 2010 at 9:02
Attachments:
Applied in r3465, thanks!
Original comment by aggraef@gmail.com
on 22 Jun 2010 at 2:18
Original issue reported on code.google.com by
neuhau...@sigpipe.cz
on 3 May 2010 at 5:00Attachments: