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

Make ML.NET tests target net8.0 and net9.0 instead of net6.0 #7097

Closed
wants to merge 27 commits into from

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Mar 21, 2024

This is a portion of the work from #6749

This moves the tests forward to net8.0, and cleans up RemoteExecutor (which I noticed in @tarekgh's PR).

I minimized the product changes so as to not break compat (thought CPU math is changing it's TFM based on net6.0 going OOS by Nov).

Where possible I tried to use NetMinimum and NetCurrent to make this easier next time.

I'm not thrilled about all the TFM duplication in the PR pipeline. I think we should clean that up in a follow-up PR though.

ericstj added 4 commits March 21, 2024 09:39
Net6 & net7 will be out of support by the time we release.
InternalDataKind.TX  and InternalDataKind.Text have the same underlying
value.  In net8.0 we started seeing ToString return the latter.
@ericstj
Copy link
Member Author

ericstj commented Mar 22, 2024

I may go ahead and do the refactoring of the build pipeline to stop passing in the test TFM. That's a bit of a hack and it's what's causing one set of the failures here.

ericstj added 2 commits March 22, 2024 13:21
Rather than constructing the test information outside the projects make
a target within the test reponsible for this.
That allows us to avoid hardcoding paths, and directly control the
behavior for the individual test at the project level.
@ericstj
Copy link
Member Author

ericstj commented Mar 25, 2024

Ok, I rewrote how we compose the test payloads. This makes this more distributed and will give tests the ability to control this in the project and targets - and removes that complexity from the pipeline.

@ericstj ericstj requested a review from directhex March 25, 2024 22:54
@ericstj
Copy link
Member Author

ericstj commented Mar 28, 2024

Getting closer here. Most remaining failures are on Arm and need unique baseline updates. I'm thinking to add a test script used by the tests that copies any outputs on failure (to aide in updating baselines). This will also let me fix the double reporting of failures.

ericstj added 3 commits March 28, 2024 18:04
Test script saves test output on failures.

It also avoids returning non-zero when the runner completes gracefully
with failures.
ericstj added 4 commits March 29, 2024 09:33
Also cleanup setting RuntimeIdentifier since it's not required.

Techincally we could be testing .NETFramework on more architectures by
setting PlatformTarget but I don't want to open that can of worms yet
since it might require new sets of baselines.
@ericstj
Copy link
Member Author

ericstj commented Dec 16, 2024

Closing this now as it's largely taken care of with @michaelgsharp's latest PR. #7319

@ericstj ericstj closed this Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant