Skip to content

Commit

Permalink
chore: unskip mssql tests (appsmithorg#36866)
Browse files Browse the repository at this point in the history
## Description
This PR fixes the flaky mssql Junit test case by replacing the test
containers docker image to mssql server 2022 latest.

### Root cause of flakiness
MSSQL Junit test cases like `mssqlplugintest.java` and
`mssqlGetDBSchemaTest.java` recently started becoming flaky in the CI.
Sometimes they would pass, sometimes they would throw the error
`org.testcontainers.containers.ContainerLaunchException: Container
startup failed for image mcr.microsoft.com/azure-sql-edge:1.0.3`. This
was happening because MSSQL test cases create a docker container using
Azure SQL Edge 1.0.3 image. Upon further researching found two relevant
links as to why it has stopped working:
1.
https://forums.docker.com/t/sql-server-docker-container-fails-on-start-up-when-run-in-a-vm-with-ubuntu-24-04/142093
2. docker/for-mac#7368

Although second link seems to be specific for Mac where as we use Ubuntu
when running tests in CI, this
[comment](docker/for-mac#7368 (comment))
and docker forum link above states that problem is there for Ubuntu as
well. Two possible workaround suggested in above threads were:
1. Downgrade docker desktop to 4.32 
2. Instead of Azure SQL Edge use MSSQL server 2022 latest image

This PR uses the second workaround and updates docker container image
from Azure SQL Edge 1.0.3 to MSSQL server 2022 latest image.

I have triggered the server-unit-tests workflow 12 times and all times
it passed, showing no flakiness, hence we should be good to go ahead
with the fix.

Equivalent EE PR: appsmithorg/appsmith-ee#5366

Fixes appsmithorg#36774 
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11380762425>
> Commit: 4f95888
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11380762425&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Thu, 17 Oct 2024 08:33:37 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No

---------

Co-authored-by: “sneha122” <“sneha@appsmith.com”>
  • Loading branch information
sneha122 and “sneha122” authored Oct 17, 2024
1 parent 9a475a8 commit 4007bdf
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.appsmith.external.models.DatasourceStructure;
import com.zaxxer.hikari.HikariDataSource;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.MSSQLServerContainer;
import org.testcontainers.junit.jupiter.Container;
Expand All @@ -23,7 +22,6 @@
import static org.junit.jupiter.api.Assertions.assertTrue;

@Testcontainers
@Disabled
public class MssqlGetDBSchemaTest {
public static final String SQL_QUERY_TO_CREATE_TABLE_WITH_PRIMARY_KEY =
"CREATE TABLE supplier\n" + "( supplier_id int not null,\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.zaxxer.hikari.HikariDataSource;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.MSSQLServerContainer;
import org.testcontainers.junit.jupiter.Container;
Expand Down Expand Up @@ -55,7 +54,6 @@
import static org.junit.jupiter.api.Assertions.assertTrue;

@Testcontainers
@Disabled
public class MssqlPluginTest {

@SuppressWarnings("rawtypes") // The type parameter for the container type is just itself and is pseudo-optional.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ public class MssqlTestDBContainerManager {

@SuppressWarnings("rawtypes")
public static MSSQLServerContainer getMssqlDBForTest() {
return new MSSQLServerContainer<>(DockerImageName.parse("mcr.microsoft.com/azure-sql-edge:1.0.3")
.asCompatibleSubstituteFor("mcr.microsoft.com/mssql/server:2017-latest"))
return new MSSQLServerContainer<>(DockerImageName.parse("mcr.microsoft.com/mssql/server:2022-latest"))
.acceptLicense()
.withExposedPorts(1433)
.withPassword("Mssql12;3");
Expand Down

0 comments on commit 4007bdf

Please sign in to comment.