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

Upgrade to Orchard Core 1.8 (OCC-223) #411

Closed
sarahelsaig opened this issue Feb 21, 2024 · 13 comments · Fixed by #412
Closed

Upgrade to Orchard Core 1.8 (OCC-223) #411

sarahelsaig opened this issue Feb 21, 2024 · 13 comments · Fixed by #412
Assignees
Labels
enhancement New feature or request

Comments

@sarahelsaig
Copy link
Contributor

sarahelsaig commented Feb 21, 2024

After Lombiq/Open-Source-Orchard-Core-Extensions#638 is completed, upgrade OCC to Orchard Core 1.8.

Jira issue

@sarahelsaig sarahelsaig added the enhancement New feature or request label Feb 21, 2024
@github-actions github-actions bot changed the title Upgrade to Orchard Core 1.8 Upgrade to Orchard Core 1.8 (OCC-223) Feb 21, 2024
@Psichorex
Copy link
Contributor

Psichorex commented Feb 22, 2024

@sarahelsaig Is this html intentionally written this way?
After OC 1.8 this is raising a duplicate form control name html validation error. Shall we ignore or address?
SO the problem is that the name attribute has the same value on multiple places.
image

@sarahelsaig
Copy link
Contributor Author

You shouldn't get that validation error, because that rule is turned off for this repo

@Psichorex
Copy link
Contributor

Psichorex commented Feb 22, 2024

Say this to my HtmlValidationReport.txt!!!! :(
HtmlValidationReport.txt

@Psichorex
Copy link
Contributor

Well by the way it seems that the CI is only failing due this test : https://github.com/OrchardCMS/OrchardCore.Commerce/actions/runs/8005597326?pr=412

And this one failed in OSOCE for me but passed on CI so it really seems to be a flaky one.

@sarahelsaig
Copy link
Contributor Author

It's trying to copy the inherited version from UITT and the one in the test project, some race condition may be at play.

@Psichorex
Copy link
Contributor

I was missing the UITT and TT nuget package updates in the Commerce solution but after upgrading them the test still fails.
What exactly do you mean by the inherited vs test project version? Isn't it only using our module from NuGet?

@sarahelsaig
Copy link
Contributor Author

I meant this:
image
The one marked as [Content] is coming from the Lombiq.Tests.UI package, while the one market as [None] is the local file added in the csproj. Up until now the latter reliably took precedence. Looks like this is no longer the case, at least not consistently. I'm not sure how this may be fixed.

@Psichorex
Copy link
Contributor

Oh it's clear now. This is only happening locally by the way so in theory this is not ruining the CI builds.

@sarahelsaig
Copy link
Contributor Author

Ok, feel free to ignore here if it works in CI. But please open a bug issue in https://github.com/Lombiq/UI-Testing-Toolbox/.

@Piedone
Copy link
Member

Piedone commented Feb 27, 2024

You need to use Content on the file here too. But you can create a file named differently too: https://github.com/Lombiq/UI-Testing-Toolbox/blob/94f57ee63781e832d2f332eea7bdc19f3fac6ac4/Lombiq.Tests.UI/Docs/Configuration.md#html-validation-configuration

@Piedone
Copy link
Member

Piedone commented Feb 29, 2024

I don't really see my above comment addressed. Having flakyness in the test project is not good.

@sarahelsaig
Copy link
Contributor Author

It's not actually flaky, it works reliably in CI. Also it's not specific to OCC. Like I said, a UITT issue should be opened.

@Piedone
Copy link
Member

Piedone commented Feb 29, 2024

But it's not an issue as I explained above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants