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

Symbol command-line tests #2

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

Conversation

mojomojomojo
Copy link

Tests of /symsearchpath and /symcachedir

I combined a number of tests into one script. I couldn't decide which was cleaner: create a dozen scripts or put all the knowledge into a single test.

Copy link
Member

@CyberShadow CyberShadow left a comment

Choose a reason for hiding this comment

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

This is great. Thank you so much.

Let me know if you want to address the below.

Comment on lines 4 to 5
rem usage: test_stat.cmd <sleepy_file_basename> <stat_descr> <target_envvar>
rem Get <stat_descr> from Stats.txt and save its value in <target_envvar>
Copy link
Member

Choose a reason for hiding this comment

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

GitHub doesn't like these, and looking here:

https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/rem

You can't use a redirection character (< or >) or pipe (|) in a batch file comment.

seems to imply there might be a problem.

Copy link
Author

@mojomojomojo mojomojomojo Jun 6, 2022

Choose a reason for hiding this comment

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

I don't mind conforming to the docs, but in my experience, this has never caused a problem.

On my Win10 system, running a batch file with this produces no output files, errors, I can pipe text into it, redirect stdout/stdin without issue.

REM  usage: t1 <t1_arg1>
REM         t1 | t2_pipe
REM  usage  t1 <t1_in
REM  usage  t1 >t1_out

echo ABCD EFGH

set /p myvar=What is your name?
echo Hello, %myvar%

I'm curious about the circumstances under which including these chars in a REM comment would do something. Has this ever failed for you?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's some legacy from some old DOS version when rem wasn't a builtin.

Copy link
Author

Choose a reason for hiding this comment

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

I updated the scripts to avoid using <>|

scripts/get_stat.cmd Outdated Show resolved Hide resolved
set "VALUE_NAME=%~1"
set "VALUE_DATA=%~2"

reg add "HKCU\SOFTWARE\codersnotes.com\Very Sleepy" /v "!VALUE_NAME!" /t REG_DWORD /d !VALUE_DATA! /f
Copy link
Member

Choose a reason for hiding this comment

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

Why not make the type a parameter instead of having two batch files?

Copy link
Author

Choose a reason for hiding this comment

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

I think I did it for readability and to hide the fact that the data was in the registry. The calling scripts care about "config data" not registry stuff.
I can change it if you'd prefer.

tests/cmdline/test.bat Outdated Show resolved Hide resolved
@@ -6,6 +6,10 @@ set SLEEPY_SILENT_CRASH=1

for %%b in (64) do (
echo Testing %%b

rem Discard any saved config options.
call ..\..\scripts\clear_config
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to put this in the tests/run_tests.bat loop, so it's done before any test is run?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't want to fool with other tests that I couldn't (time constraints) run, like the alternate compilers or debuggers. If you'd like to see it there, I can put it there.

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