Skip to content

[MINOR] Reduce logging in MLContextTests #2432

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.

Sign up for GitHub

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

Closed
Baunsgaard wants to merge 2 commits into from
+3 −19
Closed

[MINOR] Reduce logging in MLContextTests #2432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb8890b
[MINOR] Add gitignore for metals
Baunsgaard Feb 16, 2026
1fa95a1
[MINOR] Reduce logging in MLContextTests
Baunsgaard Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ maven-eclipse.xml
# vscode
.vscode

# Metals (Scala LSP)
.metals/

# checksum files
*.crc

Expand Down
6 changes: 0 additions & 6 deletions src/test/java/org/apache/sysds/test/applications/nn/BaseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ public abstract class BaseTest extends MLContextTestBase {

private static final String ERROR_STRING = "ERROR:";

public BaseTest() {
//disable debug and trace logging in mlcontext super class since
//the nn tests execute a lot of mini-batch operations
_enableTracing = false;
}

protected void run(String name) {
run(name, false);
}
Expand Down
13 changes: 0 additions & 13 deletions src/test/java/org/apache/sysds/test/functions/mlcontext/MLContextTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@

import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.spark.api.java.JavaSparkContext;
import org.apache.spark.sql.SparkSession;
import org.apache.sysds.api.mlcontext.MLContext;
Expand Down Expand Up @@ -65,8 +63,6 @@ public abstract class MLContextTestBase extends AutomatedTestBase {

protected String testDir = null;
protected String testName = null;
protected Level _oldLevel = null;
protected boolean _enableTracing = true;

@Override
public void setUp() {
Expand All @@ -76,13 +72,6 @@ public void setUp() {

addTestConfiguration(dir, name);
getAndLoadTestConfiguration(name);

//run all mlcontext tests in loglevel trace to improve test coverage
Copy link
Contributor

@e-strauss e-strauss Feb 16, 2026

Choose a reason for hiding this comment

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

how would enabling the tracing, would help with code coverage anyways? is there conditional code on log level?

Copy link
Contributor Author

@Baunsgaard Baunsgaard Feb 17, 2026

Choose a reason for hiding this comment

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

It is a bit annoying but people do this if we are not carefull.

In essence, many places use :

if(LOG.isDebug()) {
 // some code
}

This means code coverage will be low, unless we explicity enable the logging.

We can circumvent it by having explicit logging tests, instead of just fully enabling logging.

Copy link
Contributor

@e-strauss e-strauss Feb 17, 2026

Choose a reason for hiding this comment

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

ahh true, grep gave me these stats:
sysds git:(main) grep -rnF "LOG.is" . | wc -l
401
sysds git:(main) grep -rnF "LOG.isTraceEnabled" . | wc -l
251
sysds git:(main) grep -rnF "LOG.isDebugEnabled" . | wc -l
131

//of all logging in various components
if( _enableTracing ) {
_oldLevel = Logger.getLogger("org.apache.sysds").getLevel();
Logger.getLogger("org.apache.sysds").setLevel( Level.TRACE );
}
}

@BeforeClass
Expand All @@ -96,8 +85,6 @@ public static void setUpClass() {
@Override
public void tearDown() {
super.tearDown();
if(_enableTracing)
Logger.getLogger("org.apache.sysds").setLevel( _oldLevel );
}

@AfterClass
Expand Down
Loading