Closed justinmclean closed 7 months ago
Overall Project | 62.02% -7.79% |
:green_circle: |
---|---|---|
Files changed | 59.73% | :red_circle: |
Module | Coverage | |
---|---|---|
server-common | 88.1% -7.46% |
:green_circle: |
server | 87.99% | :green_circle: |
catalog-lakehouse-iceberg | 80.5% -11.75% |
:green_circle: |
core | 74.24% -6.27% |
:green_circle: |
api | 68.43% -1.24% |
:red_circle: |
catalog-hive | 65.46% -2.06% |
:green_circle: |
common | 41.98% -0.62% |
:green_circle: |
trino-connector | 21.11% -62.32% |
:red_circle: |
Can we make an exception for wildcard imports i.e. "import·static·org.junit.jupiter.api.Assertions.*;" in tests?
@mchades @xunliu can you please help to review this.
@justinmclean is this PR ready for review? There seems no checks passed.
Please see my comment above about the wild card import. Yes, it is ready for review.
Can we make an exception for wildcard imports i.e. "import·static·org.junit.jupiter.api.Assertions.*;" in tests?
I think we should never use import *
in either main or test. @jerryshao What's your opinion?
I would suggest not making an exception for tests. Firstly, I'm not sure if this exception can be set through Graviton; Secondly, I don't see a persuasive reason to make this as an exception.
testTableColumnUpdateDatatypeNullability() FAILED
It seems that Hive does not support NOT NULL
syntax in column type.
@yuqi1129 for NOT NULL if Hive doesn't support it should we not allow it? Currently:
@yuqi1129 Looks like we still have an issue with changing columns even when the data types are the same - see testTableColumnSwap.
Currently, testTableColumnSwap is failing due to a bug, but this is now ready for review.
Currently, testTableColumnSwap is failing due to a bug, but this is now ready for review.
Could you please provide some error messages?
@justinmclean current code introduces many unrelated codes, can you please fix it, thanks.
There is no unrelated code I need to merge main to get the tests to pass and to fix conflicts.
There is no unrelated code I need to merge main to get the tests to pass and to fix conflicts.
Please use git rebase main
instead, as GitHub indicates that there are 150 files changed in this PR when there are 2 actually, which makes this PR difficult to review. @justinmclean
Should this PR be closed, since #536 was opened. @justinmclean
What changes were proposed in this pull request?
Added more end to end integration tests.
Why are the changes needed?
To confirm we have working software.
Fix: #299
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Tests run on local machine using docker. Note that currently 4 tests are failing due to bugs:
TableHiveIT > testTableDeleteUnknownColumn() FAILED TableHiveIT > testTableColumnUpdatePositionToFirst() FAILED TableHiveIT > testTableColumnUpdatePositionToLast() FAILED TableHiveIT > testTableColumnUpdateDatatypeNullability() FAILED