Skip to content

Conversation

@Thaina
Copy link
Contributor

@Thaina Thaina commented Dec 7, 2024

This is not request for registry but propose change on the RegistryTests so each registry entry would be tested separately for each case and have explicit case entry for easier investigating and rerun

@bdovaz
Copy link
Owner

bdovaz commented Dec 7, 2024

@Thaina please merge master in your branch

@Thaina Thaina force-pushed the expand-with-testcasesource branch from 66c611f to 806ce83 Compare December 7, 2024 15:26
@Thaina
Copy link
Contributor Author

Thaina commented Dec 9, 2024

Already merged

Do you think are there any concern about change test like this?
Such as, I have remove Ensure_Do_Not_Exceed_The_Maximum_Number_Of_Allowed_Versions which only do Assert.Inconclusive into Warn.If

@bdovaz
Copy link
Owner

bdovaz commented Dec 9, 2024

Let me check it this week and let you know.

The test you mention is to know which packages have more versions because there are some like AWS that makes daily builds and if we don't raise the minimum version in the registry.json file from time to time, it ends up growing infinitely causing disk space or processing/initialization time problems.

@bdovaz
Copy link
Owner

bdovaz commented Dec 11, 2024

Can you resolve the conflicts? Thanks!

@Thaina Thaina force-pushed the expand-with-testcasesource branch from 806ce83 to 9d1e6ea Compare December 11, 2024 04:19
@Thaina
Copy link
Contributor Author

Thaina commented Dec 11, 2024

@bdovaz resolved

@Thaina Thaina force-pushed the expand-with-testcasesource branch from 9d1e6ea to ed73087 Compare December 11, 2024 06:36
@bdovaz
Copy link
Owner

bdovaz commented Dec 11, 2024

  • Restore the Ensure_Do_Not_Exceed_The_Maximum_Number_Of_Allowed_Versions test.
  • Could it be that in some file you have changed EOLs? Because I see more changes than it should.

@Thaina
Copy link
Contributor Author

Thaina commented Dec 11, 2024

@bdovaz

Not only it was Ensure_Do_Not_Exceed_The_Maximum_Number_Of_Allowed_Versions redundantly load whole registry in exact same manner as Ensure_Min_Version_Is_Correct_Ignoring_Analyzers_And_Native_Libs, it also bunching all of the lib that not pass the test into one long array. I think it would be more beneficial to separate warning into each of the testcase

  • Have you use Hide WhiteSpaces already?
    image

@bdovaz
Copy link
Owner

bdovaz commented Dec 11, 2024

@bdovaz

Not only it was Ensure_Do_Not_Exceed_The_Maximum_Number_Of_Allowed_Versions redundantly load whole registry in exact same manner as Ensure_Min_Version_Is_Correct_Ignoring_Analyzers_And_Native_Libs, it also bunching all of the lib that not pass the test into one long array. I think it would be more beneficial to separate warning into each of the testcase

  • Have you use Hide WhiteSpaces already?
    image

I agree with the test you mention.

Normally I don't touch that "Hide whitespace" checkbox because if I check it I don't see this kind of problems I mention, have you checked that you haven't changed any EOL? Because that's what it looks like.

@Thaina Thaina force-pushed the expand-with-testcasesource branch 2 times, most recently from 23d2e81 to 9a86758 Compare December 11, 2024 11:42
@Thaina
Copy link
Contributor Author

Thaina commented Dec 11, 2024

have you checked that you haven't changed any EOL?

I don't really sure, I use vscode and I don't know if it saved with difference EOL than the source or not and it also not report any change except 2 lines of code that it trimming whitespace automatically

@Thaina
Copy link
Contributor Author

Thaina commented Dec 13, 2024

@bdovaz If the EOL was changed can you just edit that to merge the pull?

@Thaina Thaina force-pushed the expand-with-testcasesource branch from 9a86758 to b125686 Compare December 17, 2024 09:31
@bdovaz
Copy link
Owner

bdovaz commented Dec 22, 2024

@Thaina please don't mix PRs.... You have put changes in scripts that are not test scripts.

@Thaina Thaina force-pushed the expand-with-testcasesource branch from b125686 to 565e9c2 Compare December 22, 2024 12:15
@Thaina
Copy link
Contributor Author

Thaina commented Dec 22, 2024

@bdovaz Could you please just merge it now already

@bdovaz
Copy link
Owner

bdovaz commented Dec 22, 2024

@Thaina I have merge a branch that normalizes all the files to LF, merge the main branch again and see if we have everything ok.

@Thaina Thaina force-pushed the expand-with-testcasesource branch from 565e9c2 to d2c5763 Compare December 23, 2024 03:32
@Thaina
Copy link
Contributor Author

Thaina commented Dec 23, 2024

@bdovaz Could you please just merge it now already

@Thaina
Copy link
Contributor Author

Thaina commented Dec 23, 2024

From the last failed ci

2024-12-23T04:24:31.5108329Z Unexpected error NuGet.Protocol.Core.Types.FatalProtocolException: Unable to find version '4.9.0' of package 'Roslynator.Analyzers'.
2024-12-23T04:24:31.5109543Z   C:\Program Files (x86)\Microsoft SDKs\NuGetPackages\: Package 'Roslynator.Analyzers.4.9.0' is not found on source 'C:\Program Files (x86)\Microsoft SDKs\NuGetPackages\'.
2024-12-23T04:24:31.5110719Z   https://api.nuget.org/v3/index.json: Error downloading 'Roslynator.Analyzers.4.9.0' from 'https://api.nuget.org/v3-flatcontainer/roslynator.analyzers/4.9.0/roslynator.analyzers.4.9.0.nupkg'.
2024-12-23T04:24:31.5111521Z   End of Central Directory record could not be found.

I don't think this related to my change

@bdovaz bdovaz merged commit 7dab654 into bdovaz:master Dec 25, 2024
2 checks passed
@Thaina Thaina deleted the expand-with-testcasesource branch December 25, 2024 09:52
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