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

feat: use web components 21.0.0-alpha2, fix tests #978

Merged
merged 9 commits into from
May 17, 2021
Merged

Conversation

web-padawan
Copy link
Member

Description

  1. Updated the web components to new major
  2. Added the @vaadin/vaadin-template-renderer to bring back removed Polymer template APIs.

@pekam pekam mentioned this pull request May 12, 2021
9 tasks
@tomivirkki tomivirkki force-pushed the v21-alpha1 branch 2 times, most recently from 35a72b4 to 2684934 Compare May 12, 2021 12:59
@web-padawan
Copy link
Member Author

Remaining failures:

javascript error: arguments[0]._scrollToIndex is not a function
java.lang.AssertionError: A grid cell was expected to have text content but had none.
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at com.vaadin.flow.component.grid.it.GridTestScrollingOver100kLinesIT.lambda$toStringIsUsedForObjectSerialization$1(GridTestScrollingOver100kLinesIT.java:53)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at com.vaadin.flow.component.grid.it.GridTestScrollingOver100kLinesIT.toStringIsUsedForObjectSerialization(GridTestScrollingOver100kLinesIT.java:49)
java.lang.NullPointerException
	at com.vaadin.flow.component.grid.testbench.GridElement.getFirstVisibleRowIndex(GridElement.java:72)
	at com.vaadin.flow.component.grid.testbench.GridElement.isRowInView(GridElement.java:174)
	at com.vaadin.flow.component.grid.testbench.GridElement.getCell(GridElement.java:115)
	at com.vaadin.flow.component.grid.testbench.GridElement.getCell(GridElement.java:100)
	at com.vaadin.flow.component.grid.it.RecalculateColumnWidthsIT.columnsRecalculateAfterDataChange(RecalculateColumnWidthsIT.java:40)

And the one about lit-element import:

java.lang.AssertionError: Expected only [Deprecation] warnings in the logs
Expected: a string containing "HTML Imports"
     but: was "http://172.16.1.126:8080/VAADIN/build/vaadin-1-0efec97cdf48f1cc4870.cache.js 507:8 "The main 'lit-element' module entrypoint is deprecated. Please update your imports to use the 'lit' package: 'lit' and 'lit/decorators.ts' or import from 'lit-element/lit-element.ts'.""

@web-padawan
Copy link
Member Author

Few other failures:

java.lang.AssertionError: Item count doesn't match expected:<1501> but was:<1800>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at com.vaadin.flow.component.grid.dataview.AbstractItemCountGridIT.verifyRows(AbstractItemCountGridIT.java:99)
	at com.vaadin.flow.component.grid.dataview.ItemCountEstimateGridIT.itemCountEstimateGrid_estimateLessThanCurrentRange_estimateNotChanged(ItemCountEstimateGridIT.java:102)
java.lang.AssertionError: Grid should be able to scroll after changing to defined size expected:<517> but was:<516>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at com.vaadin.flow.component.grid.dataview.ItemCountUnknownGridIT.undefinedSizeGrid_switchesToDefinedSize_sizeChanges(ItemCountUnknownGridIT.java:84)
org.junit.ComparisonFailure: Cell content should have not updated expected:<[EDITED]> but was:<[0]>
	at org.junit.Assert.assertEquals(Assert.java:117)
	at com.vaadin.flow.component.grid.it.GridItemRefreshPageIT.dataProviderRefreshItem_gridDoesntTouchOtherItemsOnDOM(GridItemRefreshPageIT.java:92)

@vlukashov vlukashov added the no-polymer Removing Polymer from Vaadin public APIs label May 14, 2021
@tomivirkki tomivirkki force-pushed the v21-alpha1 branch 3 times, most recently from f730e4d to a2f0932 Compare May 14, 2021 09:11
@tomivirkki tomivirkki force-pushed the v21-alpha1 branch 2 times, most recently from 2dcf077 to 4035eb2 Compare May 14, 2021 13:30
Comment on lines -102 to +105
verifyRows(1501);
verifyRows(1800);

setEstimate(1600);
verifyRows(1600);
setEstimate(1700);
verifyRows(1700);
Copy link
Member

Choose a reason for hiding this comment

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

The previous tests were written with the assumption that the scrollToIndex wouldn't continue to the desired index once the last scrollable index before increasing the range has been reached. This works as expected with the new version so the grid actually ends up at index 1500 after scrollToRow(1500), even if the effective size is increased dynamically.

Comment on lines -83 to +84
int expectedLastItem = 517;
int VIEWPORT_ITEM_CAPACITY = 16;
int expectedLastItem = 500 + VIEWPORT_ITEM_CAPACITY;
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 99 to 100
Assert.assertEquals(999, grid.getLastVisibleRowIndex());
Assert.assertEquals(1000 + VIEWPORT_ITEM_CAPACITY, grid.getLastVisibleRowIndex());
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines -50 to +56
String slotName = vgcc.getAttribute("slot")
.replace("vaadin-grid-cell-content-", "");
if (Integer.parseInt(slotName) <= 180) {
TestBenchElement slot = vgcc.getPropertyElement("assignedSlot");
if (headerSlots.contains(slot) || bodySlots.contains(slot)) {
Copy link
Member

Choose a reason for hiding this comment

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

This test relied the cell content elements in the light DOM belonging to body and header starting from an arbitrary index 180.

Comment on lines -1090 to +1096
waitUntil(driver -> getCells(grid).stream()
.filter(cell -> text.equals(cell.getText())).findFirst()
.isPresent());
waitUntil(driver -> {
List<?> cellContentTexts = (List<?>) getCommandExecutor()
.executeScript(
"return Array.from(arguments[0].querySelectorAll('vaadin-grid-cell-content')).map(cell => cell.textContent)",
grid);
return cellContentTexts.contains(text);
});
Copy link
Member

Choose a reason for hiding this comment

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

For some reason getting the cells via Selenium API resulted in different set than what querySelectorAll returns. Fixed to use pure JS approach instead.

"return arguments[0]._firstVisibleIndex+arguments[0]._vidxOffset",
grid);
"return arguments[0]._firstVisibleIndex", grid);
Copy link
Member

Choose a reason for hiding this comment

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

_vidxOffset is now a concept inside the virtualizer and doesn't concern grid. Now _firstVisibleIndex includes the offset value (it's still not public API!)

@web-padawan web-padawan changed the title feat: update web components to 21.0.0-alpha1 feat: use web components 21.0.0-alpha2, fix tests May 17, 2021
@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 222 issues

  • CRITICAL 46 critical
  • MAJOR 37 major
  • MINOR 128 minor
  • INFO 11 info

Top 10 issues

  1. CRITICAL AvatarGroup.java#L395: Define a constant instead of duplicating this literal "The translation should not be null" 3 times. rule
  2. CRITICAL GeneratedVaadinCheckbox.java#L226: Define a constant instead of duplicating this literal "indeterminate" 3 times. rule
  3. CRITICAL GeneratedVaadinCheckbox.java#L287: Define a constant instead of duplicating this literal "checked" 5 times. rule
  4. CRITICAL GeneratedVaadinCheckboxGroup.java#L297: Define a constant instead of duplicating this literal "invalid" 3 times. rule
  5. CRITICAL GeneratedVaadinCheckboxGroup.java#L384: Define a constant instead of duplicating this literal "value" 4 times. rule
  6. CRITICAL GeneratedVaadinComboBox.java#L430: Define a constant instead of duplicating this literal "opened" 3 times. rule
  7. CRITICAL GeneratedVaadinComboBox.java#L623: Define a constant instead of duplicating this literal "filter" 3 times. rule
  8. CRITICAL GeneratedVaadinComboBox.java#L656: Define a constant instead of duplicating this literal "selectedItem" 3 times. rule
  9. CRITICAL GeneratedVaadinComboBox.java#L860: Define a constant instead of duplicating this literal "invalid" 3 times. rule
  10. CRITICAL GeneratedVaadinComboBox.java#L1423: Define a constant instead of duplicating this literal "value" 4 times. rule

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-polymer Removing Polymer from Vaadin public APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants