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

runm3uatest: Avoid having to use a ~/.guile script #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

runm3uatest: Avoid having to use a ~/.guile script #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .guile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dotguile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why .guile is showing up here. This file does not exists in this dir and should not be here.

2 changes: 1 addition & 1 deletion dotguile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
;;; $Id: dotguile,v 1.1 2012/08/26 21:06:27 tuexen Exp $

;;; Change the following line to reflect where the files are located.
(define dir "/Users/tuexen/Documents/m3ua-testtool/")
(define dir "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep the directory name. On the one hand side it shows what to insert, on the other handside it breaks very early if you haven't changed it to an appropriate way.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it somewhat strange and uncommon that the personal home directory of the developer is hard-coded in some files that are distributed with a project. That's why I was suggesting to move to a "all relative paths' setup where no installation is required, no files to the home directory need to be deployed, etc.

If the home directory must be present in this file, then there should be some template / pattern substitution mechanism that uses the actual "$HOME" at the time of installation.

(define files (list "common.scm"
"m3ua.scm"
"m3ua-asp-tests.scm"
Expand Down
2 changes: 1 addition & 1 deletion run-some-asp-tests
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ testcases='m3ua-asp-aspsm-v-002

for testcase in $testcases
do
runm3uatest -t $timeout $testcase 2> /dev/null
./runm3uatest -d . -t $timeout $testcase 2> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires the binary to be in the directory where the script is. I assume that this is installed, which is also the case when running the tests manually.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requiring installation of a helper script to some system PATH seems like a relatively odd choice for a test tool. 99.9% of the FOSS software I know of can be executed from within the checked out git repo / build directory, without requiring system-wide installation. I see that a s a strong benefit, avoiding to clutter my system usr/local/bin etc. with tons of cruft over time.

You're of course free to reject it. With the python-based altrnative executor of the other patch we at osmocom don't even use runm3uatest anymore. It was just something developed at an earlier stage trying to make the nplab-tests run for the first time some 4 years ago.

sleep $sleeptime
done
2 changes: 1 addition & 1 deletion run-some-sgp-tests
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ testcases='m3ua-sgp-aspsm-v-003

for testcase in $testcases
do
runm3uatest -t $timeout $testcase 2> /dev/null
./runm3uatest -d . -t $timeout $testcase 2> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above...

sleep $sleeptime
done
12 changes: 8 additions & 4 deletions runm3uatest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,11 @@ main(int argc, char *argv[]) {
unsigned int timeout;
int status, c;
char command[COMMAND_LENGTH];
char *dir = getenv("HOME");

timeout = TIMEOUT;

while ((c = getopt(argc, argv, "t:")) != -1) {
while ((c = getopt(argc, argv, "t:d:")) != -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep the options in alphabetic order.

switch(c) {
case 'h':
print_usage();
Expand All @@ -83,24 +84,27 @@ main(int argc, char *argv[]) {
case 't':
timeout = (unsigned int)atoi(optarg);
break;
case 'd':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the options in alphabetic order.

dir = optarg;
break;
default:
print_usage();
return (1);
}
}

if (optind == argc - 1) {
snprintf(command, COMMAND_LENGTH, command_skel, getenv("HOME"), argv[optind]);
snprintf(command, COMMAND_LENGTH, command_skel, dir, argv[optind]);
} else {
print_usage();
return (1);
}

if ((pid = fork()) == 0) {
#if defined(__APPLE__) || defined(__FreeBSD__)
execlp("/usr/local/bin/guile", "guile", "-c", command, NULL);
execlp("/usr/local/bin/guile", "guile", "-L", dir, "-c", command, NULL);
#else
execlp("/usr/bin/guile", "guile", "-c", command, NULL);
execlp("/usr/bin/guile", "guile", "-L", dir, "-c", command, NULL);
#endif
return (255);
}
Expand Down