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

Updates to Windows tests (RunTest.bat) #468

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

alexdowad
Copy link
Contributor

The comments in RunTest.bat state that @zherczeg added 16-bit and 32-bit support... could he confirm that 544dfb0 reflects his intentions correctly?

I was able to run the CI jobs in my forked repo and the Windows 32-bit job passed. I guess that's a good thing... though since I have never seen it fail, I don't really know for sure that it is testing anything 😅

Copy link
Contributor

@carenas carenas left a comment

Choose a reason for hiding this comment

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

To help test (through the CI) I am suggesting to merge alexdowad#3 on top.

you might want to at least squash the RunTest.bat changes on your commit though if want to keep them all as independent commits

@PhilipHazel
Copy link
Collaborator

Is this ready now?

@carenas
Copy link
Contributor

carenas commented Sep 12, 2024

Is this ready now?

Tested locally as well using the command line but it seems to fail with the GUI, not sure if it is a regression though.

@alexdowad
Copy link
Contributor Author

Is this ready now?

Yes!

@carenas
Copy link
Contributor

carenas commented Sep 13, 2024

Is this ready now?

Tested locally as well using the cmdline but it seems to fail with MSVC, not sure if it is a regression though.

Indeed, not a regression, but fixing tests for the 32-bit library when the 16-bit library was not enabled

@PhilipHazel PhilipHazel merged commit 5e75d9b into PCRE2Project:master Sep 13, 2024
12 checks passed
@PhilipHazel
Copy link
Collaborator

I merged; there now seems to be a test failure...

@alexdowad
Copy link
Contributor Author

I merged; there now seems to be a test failure

I'm looking into it. Thanks for mentioning.

@alexdowad
Copy link
Contributor Author

Hmm. I can't see any failure in the GitHub Actions tab. Is there somewhere else where I can see it?

@alexdowad alexdowad deleted the batfile branch September 13, 2024 11:24
@carenas
Copy link
Contributor

carenas commented Sep 13, 2024

Hmm. I can't see any failure in the GitHub Actions tab. Is there somewhere else where I can see it?

Should be visible here (the red X next to the last commit in the "code menu")

The failure wasn't caused by this code though but just a badly timed deprecation.

@alexdowad: are you still working in a fix?, the changes to the scorecard job included in this commit seem to work from my own tests, but it might be worth looking more into it to make sure it is well maintained IMHO.

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.

3 participants