bonsai
/
coreutils
5
4
Fork 2
Code Issues 38 Pull Requests 6 Projects 1 Releases 9 Activity

WIP: testing #96

Draft
emma wants to merge 15 commits from testing into main
pull from: testing
merge into: bonsai:main
Conversation 2 Commits 15 Files Changed 15 +234 -7
emma commented 2024-04-24 21:00:28 +00:00
Owner
Copy Link
No description provided.
emma added the
enhancement
 label 2024-04-24 21:00:28 +00:00
emma added 5 commits 2024-04-24 21:00:28 +00:00
emma added 1 commit 2024-04-24 21:23:03 +00:00
emma added 1 commit 2024-04-24 21:25:30 +00:00
emma requested review from trinity 2024-04-24 21:27:43 +00:00
emma added 3 commits 2024-04-25 01:29:28 +00:00
emma added 1 commit 2024-04-25 23:21:58 +00:00
emma added 1 commit 2024-04-26 00:58:01 +00:00
trinity requested changes 2024-04-27 02:12:41 +00:00
trinity left a comment
Owner
Copy Link

I have some questions and concerns.

I have some questions and concerns.
tests/bonsai/false.sh
@ -0,0 +10,4 @@
. tests/bonsai/test_env
! false
! false -h
trinity commented 2024-04-27 01:57:11 +00:00
Owner
Copy Link

GNU false(1), a notoriously POSIX non-compliant implementation, would pass here, as (if I recall) it returns EXIT_FAILURE for improper usage. It shouldn't pass, though, because it prints a usage text to the standard output when usage help or version information are queried.

I think another good test would be false --help | wc -c | xargs test 0 =, though you should double check the exit statuses here before adding it.

GNU false(1), a notoriously POSIX non-compliant implementation, would pass here, as (if I recall) it returns `EXIT_FAILURE` for improper usage. It shouldn't pass, though, because [it prints a usage text to the standard output when usage help or version information are queried](https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/true.c#n36). I think another good test would be `false --help | wc -c | xargs test 0 =`, though you should double check the exit statuses here before adding it.
emma commented 2024-04-27 02:53:33 +00:00
Author
Owner
Copy Link

Why would we test for GNU extensions in our implementation of true(1)?

Why would we test for GNU extensions in our implementation of `true(1)`?
trinity commented 2024-04-27 13:25:37 +00:00
Owner
Copy Link

It's testing for bloat. It drives home the fact that we will not compromise on making good, simple tools. If we did our tests would fail us.

It's testing for bloat. It drives home the fact that we *will not* compromise on making good, simple tools. If we did our tests would fail us.
emma commented 2024-04-27 19:04:00 +00:00
Author
Owner
Copy Link

But if we changed our minds, we could just change our test, and if someone made a PR with changes like that, they would just change the test too.

But if we changed our minds, we could just change our test, and if someone made a PR with changes like that, they would just change the test too.
trinity commented 2024-04-29 02:04:10 +00:00
Owner
Copy Link

That's true. I suppose such a test isn't necessary.

That's true. I suppose such a test isn't necessary.
tests/bonsai/true.sh
@ -0,0 +1,13 @@
#!/bin/sh
trinity commented 2024-04-27 02:00:28 +00:00
Owner
Copy Link

Why is this separate from tests/posix-compat/true.sh?

Why is this separate from `tests/posix-compat/true.sh`?
emma commented 2024-04-27 02:52:44 +00:00
Author
Owner
Copy Link

This is true(1) from Bonsai and the one in posix-compat/true.sh is true(1p)

This is `true(1)` from Bonsai and the one in `posix-compat/true.sh` is `true(1p)`
trinity commented 2024-04-27 13:23:37 +00:00
Owner
Copy Link

Our true(1) and false(1) implementations are POSIX without extensions and shouldn't be extended. Maybe it would be better to only have them in the POSIX compatibility section.

Our true(1) and false(1) implementations are POSIX without extensions and shouldn't be extended. Maybe it would be better to only have them in the POSIX compatibility section.
emma commented 2024-04-27 19:06:20 +00:00
Author
Owner
Copy Link

It makes sense to have them in both because the true(1p) test script leverages our script; however, I have just realized that I have implemented the POSIX test suite in a different way than I originally imagined and will make more changes before we can continue this conversation.

It makes sense to have them in both because the true(1p) test script *leverages* our script; however, I have just realized that I have implemented the POSIX test suite in a different way than I originally imagined and will make more changes before we can continue this conversation.
tests/test.sh
@ -0,0 +17,4 @@
exit 1
fi
printf "Starting Bonsai testing.\n\n"
trinity commented 2024-04-27 02:10:57 +00:00
Owner
Copy Link

This seems ineligant. Programmatically print the test suite name at tests/$suite/Name, falling back to the folder name itself.

for suite in tests/*; do
    if ! scrut -d "$suite"
        then continue
    fi
    s="$suite" sh -c '
            scrut -e "$s"/Name \
                && mm <"$s/Name" \
                || printf "%s\n" "$s"'
        | xargs printf 'Testing <%s>...\n'
    # testing stuff
done
This seems ineligant. Programmatically print the test suite name at `tests/$suite/Name`, falling back to the folder name itself. ```sh for suite in tests/*; do if ! scrut -d "$suite" then continue fi s="$suite" sh -c ' scrut -e "$s"/Name \ && mm <"$s/Name" \ || printf "%s\n" "$s"' | xargs printf 'Testing <%s>...\n' # testing stuff done
tests/test.sh Outdated
@ -0,0 +28,4 @@
printf '\n'
done
if ! ls Makefile >/dev/null 2>&1
trinity commented 2024-04-27 02:12:18 +00:00
Owner
Copy Link

This should go before the previous for loop as it will crash faster on incorrect invocation.

This should go before the previous `for` loop as it will crash faster on incorrect invocation.
emma marked this conversation as resolved
trinity commented 2024-04-27 02:14:15 +00:00
Owner
Copy Link

Goodness, I just saw the WIP: . Sorry for prematurely reviewing. It makes sense then that it seemed that this was unfinished. I really like where this is headed though.

Goodness, I just saw the `WIP: `. Sorry for prematurely reviewing. It makes sense then that it seemed that this was unfinished. I really like where this is headed though.
emma added 1 commit 2024-04-27 02:56:29 +00:00
emma commented 2024-04-27 02:56:52 +00:00
Author
Owner
Copy Link

Goodness, I just saw the WIP: . Sorry for prematurely reviewing. It makes sense then that it seemed that this was unfinished. I really like where this is headed though.

I requested it.

> Goodness, I just saw the `WIP: `. Sorry for prematurely reviewing. It makes sense then that it seemed that this was unfinished. I really like where this is headed though. I requested it.
emma added 1 commit 2024-04-27 21:17:53 +00:00
emma added 1 commit 2024-04-27 21:19:04 +00:00
This pull request has changes conflicting with the target branch.
  • Makefile
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b testing main
git pull origin testing

Step 2:

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff testing
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
trinity
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
joke

you are silly!

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
duplicate
enhancement
help wanted
invalid
joke
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: bonsai/coreutils#96
No description provided.
Delete Branch "testing"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?