Skip to content

Conversation

@samisuteria
Copy link
Contributor

No description provided.

@NeedleInAJayStack NeedleInAJayStack self-assigned this May 22, 2025
@NeedleInAJayStack
Copy link
Member

These changes look great to me! Thanks Sami!

To fix CI, I think if you change this line to:

      uses: actions/upload-artifact@v4

that might fix it. Let me know if not and I can dive deeper! Thanks again!

@samisuteria
Copy link
Contributor Author

I tired debugging this with docker locally and it kept getting stuck when running swift test --parallel --enable-code-coverage I'm not sure whats going on. I can keep looking.

@samisuteria
Copy link
Contributor Author

I was able to get CI to pass by taking out the --parallel flag but the Upload test coverage html step didn't upload anything. https://github.com/GraphQLSwift/Graphiti/actions/runs/15216390715/job/42802811029?pr=150#step:6:10

I'm not sure if this was working in the past. https://codeclimate.com/github/GraphQLSwift/Graphiti/builds

@NeedleInAJayStack
Copy link
Member

NeedleInAJayStack commented May 25, 2025

I was able to get CI to pass by taking out the --parallel flag but the Upload test coverage html step didn't upload anything. https://github.com/GraphQLSwift/Graphiti/actions/runs/15216390715/job/42802811029?pr=150#step:6:10

Ah good find! Thanks for digging through these issues.

I have been hearing that code coverage has been having trouble on XCTests in Swift 6.1 in threads like this. I am able to recreate these issues in a docker container, and swift test --parallel and swift test --enable-code-coverage work, but swift test --parallel --enable-code-coverage does not. Unfortunately, I believe there's a separate bug that --enable-code-coverage requires XCTests to be executed in parallel to create it's output.

I think our best option is actually to keep --parallel but remove --enable-code-coverage, and comment out the Get test coverage html and Upload test coverage html steps with a comment that says code coverage is troublesome in parallel on Swift 6.1. Does that sound good to you?

Copy link
Member

@NeedleInAJayStack NeedleInAJayStack left a comment

Choose a reason for hiding this comment

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

Thanks!!

@NeedleInAJayStack NeedleInAJayStack merged commit cedafbc into GraphQLSwift:main Jun 7, 2025
7 checks passed
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