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

fix: run setBean validation for changed bindings only (#18735) #18760

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

tepi
Copy link
Contributor

@tepi tepi commented Feb 20, 2024

No description provided.

@tepi tepi linked an issue Feb 20, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Feb 20, 2024

Test Results

11 files   - 11  11 suites   - 11   6m 29s ⏱️ - 1m 47s
33 tests  -  8  33 ✅  -  6  0 💤  - 2  0 ❌ ±0 
29 runs   - 16  29 ✅  - 14  0 💤  - 2  0 ❌ ±0 

Results for commit d21b865. ± Comparison against base commit 973b28e.

This pull request removes 41 and adds 33 tests. Note that renamed tests count towards both.
com.vaadin.flow.custom.CustomRouteIT ‑ CustomRegistry_hasExpectedErrorHandlers[any_Chrome_]
com.vaadin.flow.custom.CustomRouteIT ‑ testCustomErrorView[any_Chrome_]
com.vaadin.flow.devbuild.DevBundleThemeIT ‑ noVaadinSegmentInTheURL_notFound[any_Chrome_]
com.vaadin.flow.devbuild.DevBundleThemeIT ‑ serveStylesInExpressBuildMode_assetsAreCopiedToBundle[any_Chrome_]
com.vaadin.flow.devbuild.DevBundleThemeIT ‑ serveStylesInExpressBuildMode_changeStyles_stylesUpdatedWithoutBundleRecompilation[any_Chrome_]
com.vaadin.flow.devbuild.DevBundleThemeIT ‑ themeAndParentThemeLoaded[any_Chrome_]
com.vaadin.flow.devbuild.DevBundleThemeIT ‑ themeComponentsCssAddedToStats[any_Chrome_]
com.vaadin.flow.devbuild.DevBundleThemeIT ‑ themeJsonContentAddedToStats[any_Chrome_]
com.vaadin.flow.devbuild.ParentThemeIT ‑ parentTheme_stylesAppliedFromParentTheme[any_Chrome_]
com.vaadin.flow.devbuild.ParentThemeInFrontendIT ‑ parentThemeInFrontendFolder_stylesAppliedFromParentTheme[any_Chrome_]
…
com.vaadin.flow.NavigationIT ‑ testNavigationPostpone_anchor[any_Chrome_]
com.vaadin.flow.NavigationIT ‑ testNavigationPostpone_routerLink[any_Chrome_]
com.vaadin.flow.NavigationIT ‑ testNavigation[any_Chrome_]
com.vaadin.flow.StateIT ‑ validateReactInUse[any_Chrome_]
com.vaadin.flow.testnpmonlyfeatures.bytecodescanning.ByteCodeScanningIT(production) ‑ buttonIsNotInitializedInProductionMode[any_Chrome_]
com.vaadin.flow.testnpmonlyfeatures.bytecodescanning.LazyIT(production) ‑ lazyLoadedWhenEnteringLazyView[any_Chrome_]
com.vaadin.flow.testnpmonlyfeatures.nobuildmojo.MultipleNpmPackageAnnotationsIT ‑ lazyComponentShouldExistInBody[any_Chrome_]
com.vaadin.flow.testnpmonlyfeatures.nobuildmojo.MultipleNpmPackageAnnotationsIT ‑ pageShouldContainTwoPaperComponents[any_Chrome_]
com.vaadin.flow.uitest.ui.ThemeEditorIT ‑ testButton[any_Chrome_]
com.vaadin.flow.uitest.ui.ThemeEditorIT ‑ testCheckbox[any_Chrome_]
…

♻️ This comment has been updated with latest results.

@tepi
Copy link
Contributor Author

tepi commented Feb 20, 2024

Once reviewed, needs to be picked to 24.2 and 24.3

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@ZheSun88 ZheSun88 merged commit 76b862a into main Feb 20, 2024
26 checks passed
@ZheSun88 ZheSun88 deleted the fix/setbean-validation branch February 20, 2024 16:16
vaadin-bot added a commit that referenced this pull request Feb 20, 2024
@knoobie
Copy link
Contributor

knoobie commented Feb 20, 2024

@tepi @mcollovati This is such a fundamental function, should there be one or multiple tests to verify that it's going to work in the future?

vaadin-bot added a commit that referenced this pull request Feb 20, 2024
@mcollovati
Copy link
Collaborator

@knoobie, my bad.
We discussed with Teppo about adding tests, but I mistakenly approved the PR too early, and it got merged.
My apologies.
We will add necessary tests in a new PR.

@knoobie
Copy link
Contributor

knoobie commented Feb 20, 2024

Don't worry, just wanted to mention it cause all my application rely on this 😬

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vaadin Flow 24.4.0.alpha4 validation process
5 participants