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

Adding system tests #399

Merged
merged 39 commits into from
Feb 11, 2025
Merged

Adding system tests #399

merged 39 commits into from
Feb 11, 2025

Conversation

Vahila
Copy link
Member

@Vahila Vahila commented Jan 20, 2025

This PR has following changes:

  • Added system tests under src/test/java/com/mathworks/ci/systemTests
  • Moved the integ test under the above mentioned folder as sometimes there are errors because of inconsistent directory and package path
  • Deleted some unnecessary tests

System tests needs access to MATLAB to run successfully. The MATLAB root path is read from 'MATLAB_ROOT' env variable. These tests are configured to run only if the 'MATLAB_ROOT' variable is defined. This is to ensure that the tests pass in Jenkinsci jobs and it makes it easier to run the tests locally as well.

Matrix project needs two versions of MATLAB to run successfully. Since there is an error while using Set up MATLAB twice, these tests would be skipped(Updating the main.yml to define MATLAB_ROOT_22b would trigger the tests)

System tests follow the standard integration tests format *IT.java. These tests can be run in isolation using failsafe plugin.
mvn test - Runs only unit tests
mvn verify - Runs both unit and system tests
mvn failsafe:integration-test - Runs only system tests

@Vahila Vahila marked this pull request as ready for review January 20, 2025 16:18
Copy link
Member

@sameagen-MW sameagen-MW left a comment

Choose a reason for hiding this comment

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

Looks really good! Definitely a huge improvement, just a couple of things that I caught during the review.

Copy link
Member

@sameagen-MW sameagen-MW left a comment

Choose a reason for hiding this comment

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

:shipit:

Looks really clean!

public void verifyGlobalToolScriptedPipeline() throws Exception {
Utilities.setMatlabInstallation("MATLAB_PATH_1", Utilities.getMatlabRoot(), jenkins);
String script = "node {\n" +
" def matlabver\n" +
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this definition here?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you are talking about 'setMATLABInstallation' this is equivalent to defining MATLAB as global tool. As in the test we are using "matlab 'MATLAB_PATH_1'", we need to define the tool first.

// 'test' task fails
jenkins.assertBuildStatus(Result.FAILURE, build);
jenkins.assertLogContains("buildtool check test dummy", build);
jenkins.assertLogNotContains("In check task", build);
Copy link
Member

Choose a reason for hiding this comment

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

I'm somewhat curious why the log shouldn't contain the output from the check task?

Copy link
Member Author

Choose a reason for hiding this comment

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

The test data is set so that the 'test' task fails(as source folder is not added to path). This is so that we have a task that fails which can used in testing different scenarios

@Vahila Vahila requested a review from mw-kapilg February 11, 2025 06:14
@Vahila Vahila merged commit 707dece into dev_main Feb 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants