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

HPCC4J-594 Add support for skipping tests #710

Open
wants to merge 2 commits into
base: candidate-9.2.x
Choose a base branch
from

Conversation

jpmcmu
Copy link
Contributor

@jpmcmu jpmcmu commented May 17, 2024

  • Updated version of surefire plugin to support excludes file
  • Added logic to generate an excludes file to GH actions

Signed-off-by: James McMullan [email protected]

Type of change:

  • This change is a bug fix (non-breaking change which fixes an issue).
  • This change is a new feature (non-breaking change which adds functionality).
  • This change is a breaking change (fix or feature that will cause existing behavior to change).

Checklist:

  • I have created a corresponding JIRA ticket for this submission
  • My code follows the code style of this project.
    • I have applied the Eclipse code-format template provided.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the HPCC Systems CONTRIBUTORS document (https://github.com/hpcc-systems/HPCC-Platform/wiki/Guide-for-contributors).
  • The change has been fully tested:
    • This change does not cause any existing JUnits to fail.
    • I have include JUnit coverage to test this change
    • I have performed system test and covered possible regressions and side effects.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Premature optimization
    • This change fixes the problem, not just the symptom

Testing:

@jpmcmu jpmcmu requested a review from rpastrana May 17, 2024 13:12
Copy link

Jira Issue: https://hpccsystems.atlassian.net/browse/HPCC4J-594

Jirabot Action Result:
Workflow Transition: Merge Pending
Updated PR

@drealeed
Copy link
Collaborator

another issue is the ecl generation code that generates the files failing for some reason against the 160 and other clusters. Rodrigo had me comment it out locally in order to get my tests to run. Do you know if your changes will address that too?

@rpastrana
Copy link
Member

Hi @drealeed those aren't necessarily related to this change, can you write up the executeECLScript problem? Thanks

Copy link
Member

@rpastrana rpastrana left a comment

Choose a reason for hiding this comment

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

@jpmcmu a few questions.

- name: Build with Maven
run: mvn -B --activate-profiles jenkins-on-demand -Dmaven.gpg.skip=true -Dmaven.javadoc.skip=true -Dmaven.test.failure.ignore=false -Dhpccconn=https://eclwatch.default:8010 -Dwssqlconn=https://sql2ecl.default:8510 -DHPCC30117=open install
Copy link
Member

Choose a reason for hiding this comment

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

let's make sure all references to HPCC30117 are removed in code

@@ -34,6 +34,7 @@
<value>org.hpccsystems.ws.client.TestResultNotifier</value>
</property>
</properties>
<excludesFile>../excluded-tests.txt</excludesFile>
Copy link
Member

Choose a reason for hiding this comment

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

it seems ok to have a single excluded-tests file at the hpcc4j level, but I could also see an argument for dedicated excluded-tests files per sub project.
what are the main drawbacks/advantages of the current implementation?

@@ -45,7 +45,7 @@
<maven.deploy.version>2.8.2</maven.deploy.version>
Copy link
Member

Choose a reason for hiding this comment

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

can we have a sample entry in the above excluded-tests.txt file?
Perhaps create a new test simply for the purpose of always excluding it via this file?
Also, does the github action mechanism append excluded tests entries, or does it overwrite the file?

import org.junit.Test;
import org.junit.experimental.categories.Category;

@Ignore
Copy link
Member

Choose a reason for hiding this comment

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

@drealeed I'd like to integrate these tests into the standard wsclient tests, is there anything RAMPS specific being tested? If so, could you provide ECL to recreate the structures being testsed here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, this was just a way to get a really comprehensive test of all possible record structures; I picked ramps dev because it had a lot of files for the regression test and I had access to it. And at the time the ecl errors occurred reading files from ramps dev so I pointed the test at the previously failing files to make sure they worked now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately the files tested in testFileMetadata() no longer exist. That test can be removed. We have similar tests in tardis for different hpcc ecl structures now.

Copy link
Member

Choose a reason for hiding this comment

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

Unless those ecl structures are proprietary, I'd like to absorb as many tests as possible. Does it make sense to donate those tests given the fact we can preemptively create the target files via ecl scripts?

Copy link
Member

Choose a reason for hiding this comment

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

@drealeed do you recall how test::wsclient::noecl was created? it seems like an edge case we should continue to test

- Updated version of surefire plugin to support excludes file
- Added logic to generate an excludes file to GH actions
- Separated test subproject test runs in GH actions
- Removed old tests
- Fixed surefire upgrade issues
- Added excluded-tests.txt file with examples

Signed-off-by: James McMullan [email protected]
@jpmcmu jpmcmu closed this May 29, 2024
@jpmcmu jpmcmu reopened this May 29, 2024
Copy link

Jira Issue: https://hpccsystems.atlassian.net/browse/HPCC4J-594

Jirabot Action Result:

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.

3 participants