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

giant rewrite of cucumber #1113

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

giant rewrite of cucumber #1113

wants to merge 16 commits into from

Conversation

al-niessner
Copy link
Contributor

@al-niessner al-niessner commented Jan 15, 2025

🗒️ Summary

Complete rewrite of cucumber testing.

⚙️ Test Data and/or Report

All unit tests should pass and there should be 251 cucumber tests.

♻️ Related Issues

Closes #1102

Reduced the content in any given line to just the essentials. Use of quotes in cucumber is odd but workable. Allowing some columns to be blank was not intuitive but is also workable. The line is much shorter with little to know repetitive information. The checks are more complete and StepDefs is completely cleaned up. Weill, probably not completely but good enoough for next step.
@al-niessner al-niessner self-assigned this Jan 15, 2025
@al-niessner al-niessner requested a review from a team as a code owner January 15, 2025 22:21
@al-niessner al-niessner marked this pull request as draft January 15, 2025 22:21
@al-niessner
Copy link
Contributor Author

@jordanpadams @nutjob4life @tloubrieu-jpl

Look at the output of cucumber (follow github action links) to make sure the output is sufficient. As you can see by the code changes a lot has changed - mostly deleted.

My next step (maybe tomorrow but probably next week) is to write a script that converts the old tables to the new ones. I am keeping the multiple files for no good reason. I do not like having to grep the feature file to find the test that is failing. I like having the smaller files. Usually when I am in the feature file, I am actively working the bug so I know which file to find it in. Hence no good reason to go any direction other than to leave it as is.

@nutjob4life
Copy link
Member

@al-niessner I can't comment if the output looks okay but I can celebrate code deletion and also this:

[INFO] Results:
[INFO] 
[WARNING] Tests run: 25, Failures: 0, Errors: 0, Skipped: 23
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

So woot woot! 🎉

Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

Visual inspection: ✓
Maven tests: ✓

[INFO] Results:
[INFO] 
[WARNING] Tests run: 25, Failures: 0, Errors: 0, Skipped: 23
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

Then produced output from validate command should be similar to reference <refOutputValue> or no error reported.

@v3.6.x
Feature: 3.6.x
Copy link
Member

Choose a reason for hiding this comment

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

@al-niessner looks great! a couple comments:

  • will need a testId column in the feature file. this ties to our I&T system and ensure we have a unique identifier for each test that traces back to ticket. @tloubrieu-jpl can maybe provide some more details about what is expected there, but I am pretty sure a testId column is needed.
  • I would prefer we keep the tags in the files, if possible, so you can run mvn test for a specific subset of tests, e.g. -Dcucumber.filter.tags='@pre.v3.6.x'
  • @tloubrieu-jpl, do we need the a specific feature name associated with these tests for TestRail, or does it not matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jordanpadams

  1. testId still exists but comes in two parts: column 1 and column 2 if first index is 1. The scenerio outline contains all of the boilerplate and joins those two columns. In eclipse it looks like:
Scenario Outline: NASA-PDS/validate#1028 1                                                                                                                                     �[90m# src/test/resources/features/3.6.x.feature:9�[0m
  �[32mGiven �[0m�[32man �[0m�[32m�[1m1028�[0m�[32m, �[0m�[32m�[1m1�[0m�[32m, and �[0m�[32m�[1m"github1028"�[0m                                                                                                                                           �[90m# cucumber.StepDefs.an_and(java.lang.Integer,java.lang.Integer,java.lang.String)�[0m
  �[32mWhen �[0m�[32mexecute validate with �[0m�[32m�[1m"--skip-context-validation -t {datasrc}/xa.s16..shz.1976.070.0.xml --schema {datasrc}/PDS4_PDS_1N00.xsd --schematron {datasrc}/PDS4_PDS_1N00.sch"�[0m �[90m# cucumber.StepDefs.execute_validate(java.lang.String)�[0m
  �[32mThen �[0m�[32mcompare to the .�[0m                                                                                                                                                        �[90m# cucumber.StepDefs.compare_to_the()�[0m

Scenario Outline: NASA-PDS/validate#1028 2                                                                                                                                     �[90m# src/test/resources/features/pre.3.6.x.feature:9�[0m
  �[32mGiven �[0m�[32man �[0m�[32m�[1m1028�[0m�[32m, �[0m�[32m�[1m2�[0m�[32m, and �[0m�[32m�[1m"github1028"�[0m                                                                                                                                           �[90m# cucumber.StepDefs.an_and(java.lang.Integer,java.lang.Integer,java.lang.String)�[0m
  �[32mWhen �[0m�[32mexecute validate with �[0m�[32m�[1m"--skip-context-validation -t {datasrc}/xa.s16..shz.1976.070.0.xml --schema {datasrc}/PDS4_PDS_1N00.xsd --schematron {datasrc}/PDS4_PDS_1N00.sch"�[0m �[90m# cucumber.StepDefs.execute_validate(java.lang.String)�[0m
  �[32mThen �[0m�[32mcompare to the .�[0m                                                                                                                                                        �[90m# cucumber.StepDefs.compare_to_the()�[0m

If you look at the output from github actions you will see the same thing:

Scenario Outline: NASA-PDS/validate#1028 1                                                                                                                                     # src/test/resources/features/3.6.x.feature:9
  Given an 1028, 1, and "github1028"                                                                                                                                           # cucumber.StepDefs.an_and(java.lang.Integer,java.lang.Integer,java.lang.String)
  When execute validate with "--skip-context-validation -t {datasrc}/xa.s16..shz.1976.070.0.xml --schema {datasrc}/PDS4_PDS_1N00.xsd --schematron {datasrc}/PDS4_PDS_1N00.sch" # cucumber.StepDefs.execute_validate(java.lang.String)
  Then compare to the .                                                                                                                                                        # cucumber.StepDefs.compare_to_the()

Scenario Outline: NASA-PDS/validate#1028 1                                                                                                                                     # src/test/resources/features/pre.3.6.x.feature:9
  Given an 1028, 1, and "github1028"                                                                                                                                           # cucumber.StepDefs.an_and(java.lang.Integer,java.lang.Integer,java.lang.String)
  When execute validate with "--skip-context-validation -t {datasrc}/xa.s16..shz.1976.070.0.xml --schema {datasrc}/PDS4_PDS_1N00.xsd --schematron {datasrc}/PDS4_PDS_1N00.sch" # cucumber.StepDefs.execute_validate(java.lang.String)
  Then compare to the .                                                                                                                                                        # cucumber.StepDefs.compare_to_the()
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.888 s -- in cucumber.CucumberTest

Yeah, I updated my pre.3.6.x to be subtest 2 while both are 1 in the commited files. This problem exists no matter how you do it and can use a python script to check the files that the same ticket number and subtest do not exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jordanpadams

  1. I did not know there was a purpose to that redundant information. Add it back.

Al Niessner added 13 commits January 16, 2025 14:47
Bulk of the work is done. Made the report.json match (give or take) for the tests from when run on main as well. If they are wrong, they are as wrong as they ere anyway.

Added a new test function for running single scenerios as though typed in on the command line. cut-n-paste is no longer your friend when translating scenerios to command lines. However, what it should look like is printed out so you can just grab it from there now.
@al-niessner
Copy link
Contributor Author

@jordanpadams

Please review when you can. Once this is merged, I will rebase then integrate all of the outstanding update PRs on validate.

There are 2 new tools for developing in validate. cucumber.SingleScenerio lets you run any one scenerio to check that it will pass later. The other is RunFeatureScenerioAsValidateFromCLI. It translates any one scenerio into a set of command line args then sends it off to validate as though it was called from the command line. I did both of these because cut-n-paste does not work so well in eclipse anymore.

Lastly, if we can figure out how, then we can run these in parallel. Order no longer matters - YAY!!! - and all tests are independent. I started looking at doing them in parallel then gave up quickly because all tests were not working yet. They are now, so lets schedule moving to parallel processing.

@al-niessner al-niessner marked this pull request as ready for review January 24, 2025 22:31
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.

Reformat cucumber feature files to be more succinct
3 participants