Fix CI regression test-fast skipped tests except on Windows#2342
Fix CI regression test-fast skipped tests except on Windows#2342EliahKagan merged 1 commit intoGitoxideLabs:mainfrom
test-fast skipped tests except on Windows#2342Conversation
I had failed to remove an `if` guard that was meant to be removed in a refactoring, in c1a3a99 (GitoxideLabs#2337). That made it so `test-fast` skips running the tests on non-Windows systems and just reports success! This fixes that serious CI bug by removing the left-over `if:`.
This comment was marked as outdated.
This comment was marked as outdated.
No worries about that! Regarding the "Would it have caught that stray This is where Azure and Copilot should really shine and I'd hope we can use more of it. That might mean that you can use it more for running research jobs, many of which will come out empty or garbage, but some of which will be successful and still safe time overall. At least this is how I use it, besides reviews, and it's good for that. |
I agree--we should use that together with other practices and scanning techniques, and it is often capable of catching problems of this general kind, even when other measures don't even approach doing so. However, in this case, I think the problem is harder for Copilot to notice than it seems.
The problem is that the disconnect between how I did mark this and my own intepretation of how I marked it was the mistake. I commented that having two sections with separate conditionals was bad and needed to be refactored. That was in the code before the refactor. Then, when refactoring, I removed the comment and one section, while modifying the other section as needed, except that I failed to remove its Nonetheless...
I agree with this in general. Regarding Azure, some AI-related services are nontrivial to set up. As you know, there was an opportunity a while back for me to get help with that, but unfortunately I didn't have the time to make that happen. But I think I'll probably still be able to get everything set up. For setting up Azure Pipelines to complement GitHub Actions, I don't expect any big problems, though there may be more steps to go through if we need GPU runners for anything. |
Somehow I still have a feeling that one could get such help even once the initial period is expired. It's probably worth an attempt once you know what you are after. I wouldn't even know what to ask. Overall, all good I think. |
|
One thing I want to look into is whether we can use our Azure credits to pay for GitHub-related services. My impression is that we could, but while it's definitely possible to pay for them through Azure, I'm not 100% sure the credits would get used. I'll definitely want to look into that first! (For things on Azure, I'm much less worried that any such surprises could occur--please do use the Azure services anytime you have a use for them.) Regarding Copilot, does it show you as having a Pro subscription or as having a Pro+ subscription? We both have Copilot provided by GitHub in connection with our work on gitoxide, but we actually have it through different mechanisms if I underrstand correctly, so there's no reason to expect it works the same for both of us. It shows as Pro+ for me, but there seem also to be bugs, or ways I don't understand how premium requests work, such that Copilot pull requests always fail for me on the grounds that I'm out of premium requests, even as other Copilot features continue to work. (With that said, I don't know all other Copilot features work--I mostly just use the chat interface, on the website.) |
|
Yes, I am afraid of a billing-messup as well. It's complicated, and buggy for sure. And regarding the subscription, that's interesting, it shows as GitHub Copilot Pro for me and at least I don't have any issues unless I try to launch too many. Sometimes two at a time work, often not. Maybe that's buggy as well. If in doubt, I wouldn't change a thing, but it would certainly be interesting to know if there is potential to fix it. For instance, if the billing goes through the organisation, I am pretty sure it would have to allow more parallelism. |
|
It may be possible to enable Copilot for the organization itself on this page. However, when I try to do that, it gives an error without much detail. It may be that wiring up Azure to it would fix that, but that goes back to being the situation where I'd want to look into it first to avoid charges that bypass the credits. |
I had failed to remove an
ifguard that was meant to be removed in a refactoring, in c1a3a99 (#2337). That made it sotest-fastskips running the tests on non-Windows systems and just reports success!This fixes that serious CI bug by removing the left-over
if:.I noticed this in #2341 by observing that some tests passed when they shouldn't have been able to, and very fast.
As for this PR, I think everything should pass, but the bug also made it so that the changes to
test-fastwas not adequately tested before merging #2337, so that's not guaranteed.