- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 160
 
Update workflow to run swift 6.2 on different machines #882
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
Conversation
| 
           Hello. Thanks for opening a PR on Exercism 🙂 We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in. You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch. If you're interested in learning more about this auto-responder, please read this blog post. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.  | 
    
| 
           Hello 👋 Thanks for your PR. This repo does not currently have dedicated maintainers. Our cross-track maintainers team will attempt to review and merge your PR, but it will likely take longer for your PR to be reviewed. If you enjoy contributing to Exercism and have a track-record of doing so successfully, you might like to become an Exercism maintainer for this track. Please feel free to ask any questions, or chat to us about anything to do with this PR or the reviewing process on the Exercism forum. (cc @exercism/cross-track-maintainers)  | 
    
| 
           @meatball133 would appreciate if you can reopen this PR to update commit history and checks. Looks like it works now (this are tests from my fork). Swift 6.2 building wihout issue, but there are warnings, will look for them in next PRs.  | 
    
| - name: Run tests | ||
| run: swift test | ||
| 
               | 
          ||
| run-exercises-tests-macos: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to also test outside of xcode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I fully understand the question. For CI, it’s simple to use Swift from Xcode since it’s pre-installed and comes with Xcode and macOS — version 6.2 for Xcode 26. However, it’s not using Xcode directly to run the tests. Can someone without Xcode run the tests with only the Swift toolchain installed? Probably yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand is the toolchain different (if I can recall, they use a different build system). I think it would be good for just the exercise test to test if possible both xcode and doing it outside of xcode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What value would it add to have the tests run twice as part of the CI? The purpose of the CI step is to validate the exercise tests work, not to test the Swift test runner. Adding a second test increases the cost/complexity of the CI. What concrete value does it add?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never mentioned the Swift test runner. The swift in xcode has a different build system compared to if you install just swift. In practice these behaves differently and code are not for graunted cross platform in swift thereby. Since certain libraries doesnt exsist on certain platforms. This in practice if only tested on one platform could mean it doesnt work on another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the purpose of this CI to validate the tests work or that the build systems work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It suppose to test that the exercises can be built on different build systems. If we just say that if the exercises pass on one system/setup, it should automatically work on every other system/setup, then we don't have to test on xcode/macos at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TL;DR After conducting some research, I think it’s overkill to test building an exercise both with and without Xcode. The difference isn’t in the build system (it’s the same — Swift Package Manager), but rather in the dependencies included with different versions of the toolchain: installed via Xcode, Swiftly, or the standalone .pkg installer.
Running tests without Xcode does not work out of the box after installing the toolchain. It throws errors about missing the Testing library. It also does not work — and probably will not — with Swiftly, because that distribution completely lacks the required Testing libraries, which is strange. Additionally, it is missing XCTest, which historically has not been part of Swift and it's expected.
However, the required Testing libraries are included in the .pkg installer, and tests work with some warnings:
swift test --toolchain <path-to-installed-toolchain>So, for macOS users, I think we should strongly recommend installing Swift via Xcode, which includes everything needed, and probably not even mention Swiftly in the installation guide.
Toolchain differences between Apple and non-Apple platforms
What’s even more frustrating is that the toolchain itself differs between Apple and non-Apple platforms. On Apple platforms (macOS), Foundation comes as a preinstalled Foundation.framework. This means that in your code you can simply write:
import Foundation  graph TD;
      MacOS[MacOS toolchain]-->FF
      NA[Non-Apple Platform toolchain]-->SCLF
      
      subgraph Apple InHouse
           FF[Foundation.framework]
      end
      FF-->SF
      subgraph GitHub
        SCLF[swift-corelibs-foundation]-->SF
        SF[swift-foundation]-->FICU[swift-foundation-icu]
        SF-->SC[swift-collections]
      end   
    However, on non-Apple platforms, this is not the case. These platforms use a specific lib called swift-corelibs-foundation, which encapsulates platform-specific logic and comes with the Swift toolchain (it is not included for macOS).
There is no single Foundation import that provides everything. This means that if you need XML or networking capabilities, you must import Foundation functionality as separate standalone modules.
As far as I understand, this is something Apple would eventually like to achieve on macOS as well, but the ETA is unknown.
For now, on non-Apple platforms (e.g., Linux), you are able to add the following imports:
import Foundation
import FoundationEssentials
import FoundationXML
import FoundationNetworking
import FoundationInternationalizationIt’s hard to imagine someone on a non-Apple platform including any of these imports in an exercise, but it is technically possible. If they do, it will break the build on macOS.
For this reason, I have added one CI job to test building on macOS, assuming GitHub Actions workflows are free for Exercism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I totally think we should have macOS and Linux ci, and I would like to add that I like the extra Linux ci for the generator, since I have noticed that it behaves differently on macOS and Linux.
I find your conclusion that it is too much work to add non xcode testing, so I think we will just leave it as it is like this. Good job!
What's new?
I want to address several issues with this PR:
jqusage intest_generartor.shscript withgrep.jqis not installed by default.test_generator.shscript is rewritten in a more modern and secure way.The logic behind the number of jobs
The goal is to check whether the exercises build correctly on different versions of the language. Also it checks the exercise will build on student's ubuntu machine.
The goal is to ensure the exercise builds on macOS. Some code may differ in terms of library usage (e.g., there is no
import FoundationNetworkingin macOS).For now, there are no exercises that use exotic libraries, but this could change in the future.
The goal is just to run them and test the logic, so one platform should be sufficient.
This will catch any platform specific build problem and any platform-specific scripting issues.
Honestly, I think I may have gone a bit overboard with the number of jobs. Let me know if it’s too much.