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

Draft: Allow running tests against installed rgbds #1621

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

Conversation

robbi-blechdose
Copy link
Contributor

Closes #1612

The two rgbgfx-related programs still use hardcoded paths and are thus broken. Working on that.

I'd appreciate some input on the naming of the commandline parameter (currently --use-system-binaries).

@Rangi42 Rangi42 added this to the 0.9.1 milestone Jan 18, 2025
@Rangi42 Rangi42 added tests This affects the test suite builds This affects the build process or release artifacts labels Jan 18, 2025
test/asm/test.sh Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I often run the test.sh scripts individually; it would be nice if this defaulted to ../../ if not provided (I guess you would now have to do RGBDS=path/ ./test.sh?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does default to ../../. That behavior only changes when --use-system-binaries is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, the test.sh scripts now default to the installed RGBDS:

Screen Shot 2025-01-18 at 12 45 10

That should be an opt-in feature just for Debian.

Copy link
Contributor Author

@robbi-blechdose robbi-blechdose Jan 18, 2025

Choose a reason for hiding this comment

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

Ah, I see my mistake now. The default comes from run-tests.sh, so it's missing for the individual files.
How would you propose I implement the default in the individual files?
Would passing the commandline argument (--installed-rgbds) into them be satisfactory?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think it should be kept simple.

RGBDS=../../
if [[ $# -eq 1 ]]; then
	RGBDS="$1"
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That won't quite work (the PATH version means we don't set RGBDS to anything), but I've got an idea in mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, should work now

test/run-tests.sh Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Half the CI fails for some reason (Windows, MinGW, Cygwin, and macOS static)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm looking into it but would appreciate some help..

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's because the failing CIs do not install RGBDS, they only copy the .exes.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is the reason, maybe some kind of "alias=" for those systems can make those binaries appear as the installed ones ? Or adding those to the path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds This affects the build process or release artifacts tests This affects the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run tests against installed rgbds, not on built binaries
3 participants