Skip to content

Conversation

@sbomer
Copy link
Member

@sbomer sbomer commented Feb 8, 2023

In #81439 I added logic to src/tests/build.sh that would support the same syntax as the cmd script for the test filter args. However, this isn't working correctly because the shift happens inside a sourced script, and doesn't actually shift the outer arguments. The right way to do this is via __ShiftArgs.

Without this fix, tree nativeaot was working because nativeaot happened to be a supported argument as well. With the fix, other test subtrees work as expected.

@ghost ghost assigned sbomer Feb 8, 2023
@sbomer sbomer requested a review from trylek February 8, 2023 18:42
@ghost
Copy link

ghost commented Feb 8, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

In #81439 I added logic to src/tests/build.sh that would support the same syntax as the cmd script for the test filter args. However, this isn't working correctly because the shift happens inside a sourced script, and doesn't actually shift the outer arguments. The right way to do this is via __ShiftArgs.

Without this fix, tree nativeaot was working because nativeaot happened to be a supported argument as well. With the fix, other test subtrees work as expected.

Author: sbomer
Assignees: sbomer
Labels:

area-Infrastructure-coreclr

Milestone: -

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks Sven! Please note however that @jeffhandley is right now reviewing

#81733

including a broad revamp of the script so please consider syncing up with him to incorporate your change in his, otherwise you'll end up with a huge conflict and will need to basically redo your change.

@runfoapp runfoapp bot mentioned this pull request Feb 8, 2023
@jeffhandley
Copy link
Member

Thanks for tagging me here, @trylek. I just merged #81733 and I think we'll be clear of merge conflicts.

@sbomer
Copy link
Member Author

sbomer commented Feb 9, 2023

Failure is #81859

@sbomer sbomer merged commit 232fbce into dotnet:main Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2023
@sbomer sbomer deleted the fixTestBuildArgs branch November 3, 2023 18:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants