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.

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

Test2 #5

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Test2 #5

wants to merge 13 commits into from

Conversation

3b
Copy link

@3b 3b commented Sep 13, 2020

Some utilities for exiting on errors, muffling asdf bad-system-name, and a roswell script to load a system and evaluate a form for CI without a test framework

@neil-lindquist
Copy link
Owner

Thank you for putting this together. The run-test-forms script is an elegant way to provide basic support for testing frameworks that don't have a convenient script for testing.
Sorry for the failing tests, none of them seem to be caused by your changes. Apparently, they've bit rotted and I don't have them set to run regularly.

However, there are a couple things I'm having trouble understanding the value of.

First is hiding asdf:bad-system-name warnings. The warning is intended to inform the user that asdf will fail to find the system under certain conditions and that they should adjust their system name to the supported scheme. Are there particular use cases you have in mind where it's beneficial to remove the warning? I am opposed to hiding it by default and feel that, if supported, it should only be enabled when the user explicitly enables it.

Second, is the Quicklisp wrapper. It doesn't feel like it provides much simplification but hides what the code is doing. ((ci-utils/utils:with-fail-on-errors (:code 5) (ql:quickload:foobar)) is more obviously quickload with some special behavior related to errors than (ci-utils/utils:quickload :foobar :fail-on-error 5)).

Finally, I have a couple quibbles about code style, but they're all pretty minor and can wait until the big picture things are decided (unneeded progns, captializations, etc).

@neil-lindquist
Copy link
Owner

The pull request tests should be fixed if you rebase/merge the updates to master into your branch.

@3b
Copy link
Author

3b commented Sep 14, 2020

The main use for the bad-system-name muffling is that when testing portability I want to test code even if dependencies have problems (an actual problem for pngload, where some of its dependencies have "bad" systems, but otherwise work on the implementations I want to test).
Changing the defaults is OK with me though.

The quickload wrapper currently doesn't do much, but I'd like to extend it to have finer grained checking for bad systems, and eventually fail on warnings as well. Ideally, I'd like both of those to enabled for the code being tested, but not for dependencies. Haven't quite figured out how to distinguish systems during loading though. Might need to ask asdf for a list of dependencies and ql those with one set of settings, then load the main system with other settings.

@neil-lindquist
Copy link
Owner

I didn't think about dependencies that raise the warning.
One possible approach to provide fine grained control is the *known-systems-with-bad-secondary-system-names* hash table. Unfortunately, it was only added in December 2018 and while the bad-system-name warnings were added December 2016, so users that don't want to see the warning would either need a recent or old version of ASDF. Additionally, it does make the user explicitly specify each of the problematic systems.

Providing functionality for catching compile warnings would be useful. In addition to getting dependencies from ASDF, the code will likely need to filter the dependencies based on the system's file and manually build a tree of all of the user's systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants