Skip to content

Add pkgconf test suite#119

Draft
dcbaker wants to merge 2 commits into
cps-org:mainfrom
dcbaker:submit/pkgconf-tests
Draft

Add pkgconf test suite#119
dcbaker wants to merge 2 commits into
cps-org:mainfrom
dcbaker:submit/pkgconf-tests

Conversation

@dcbaker
Copy link
Copy Markdown
Collaborator

@dcbaker dcbaker commented Feb 11, 2025

I have ported many (but not all) of the tests from pkgconf's Kyua based runner to a simple Python based runner using toml descriptions, much like the cps test runner. I have chosen to keep this somewhat separate as I'd like to allow us to more easily port changes from upstream here. Potentially with a plan to use wrap/fetchContent to download the pkgconf sources instead of vendoring the tests. We'll see.

Anyway, I haven't done the work to add the tests to CMake yet since only about 30% of them work ATM. If someone else wants to do that I can squash that in.

@lunacd
Copy link
Copy Markdown
Contributor

lunacd commented Feb 17, 2025

@dcbaker Are you working on this? Trying to look for the next thing to work on, but don't want to end up doubling the effort with you

@dcbaker
Copy link
Copy Markdown
Collaborator Author

dcbaker commented Feb 18, 2025

Not really. I was double checking/updating the port of the tests. There' some tests that are defined in upstream pkgconf, but never get run (and many fail when they are run), and see what should be done about that, but I haven't been working on make the tests pass with cps-config, I'm more interested in finishing the core cps implementation.

@dcbaker
Copy link
Copy Markdown
Collaborator Author

dcbaker commented Feb 28, 2025

One option we could do here if you wanted is to get the tests into cmake, and then add an "expected fail" status to all of the failing tests which would make it easier to incrementally fix them?

@lunacd
Copy link
Copy Markdown
Contributor

lunacd commented Mar 1, 2025

Oh yeah, good idea actually. Let me do that as my next item

@dcbaker dcbaker force-pushed the submit/pkgconf-tests branch from 8703681 to f454388 Compare March 6, 2025 18:05
I didn't do this when I rebased. I'm not convinced I actually handled
this correctly, but I'd rather set a valid but wrong value than have an
invalid value that causes crashes.
@dcbaker dcbaker force-pushed the submit/pkgconf-tests branch 3 times, most recently from 272ae22 to fda86a3 Compare March 6, 2025 18:16
There is still some work to do sorting out with upstream some tests that
are defined but not run, there are also a few tests not ported (due to
complexity of the port) but in general this is close enough to merge.
There's about 100 passes and 60 failures currently, the failures have
been marked as expected failures, and the test runner understands these
are expected failures (annotated in the toml) and reports them as passes
as long as they fail.

This is currently not plugged into the cmake.
@dcbaker dcbaker force-pushed the submit/pkgconf-tests branch from fda86a3 to 2a292b2 Compare March 6, 2025 18:20
@dcbaker
Copy link
Copy Markdown
Collaborator Author

dcbaker commented Mar 6, 2025

@lunacd I attempted the expected failure approach. looks like I may either have to make that per-platform, or we'll have to do some work on windows before we're ready to merge still.

@lunacd
Copy link
Copy Markdown
Contributor

lunacd commented Mar 9, 2025

So I just had CMake support added here: https://github.com/lunacd/cps-config/tree/submit/pkgconf-tests. I would personally vote for marking those windows failure as expected fail (and comment windows-only maybe), and then we can come around to it as separate PRs

@lunacd
Copy link
Copy Markdown
Contributor

lunacd commented Mar 9, 2025

Okay, so I did that and also relaxed the success criteria for runner.py. See lunacd@a602768. At this point I should probably PR against your PR lol

@lunacd
Copy link
Copy Markdown
Contributor

lunacd commented Mar 9, 2025

Many of them look like they have the same root cause tho. I expect we can flush them out rather quickly when I get it

@lunacd
Copy link
Copy Markdown
Contributor

lunacd commented Mar 9, 2025

looks like I may either have to make that per-platform

This will be an alternative approach for relaxing runner.py success criteria. I don't know which one is technically better. Attacking it from runner.py is definitely way easier. Also, see above comment, I don't expect we need the xfail facilities all that long. So maybe the quick and shabby way is acceptable in the short term

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