Skip to content

Conversation

@gayanW
Copy link
Contributor

@gayanW gayanW commented Aug 14, 2025

This adds a new command launchable compare subsets which can be used to compare two subset files.

 % launchable compare subsets --help
Usage: launchable compare subsets [OPTIONS] FILE_BEFORE FILE_AFTER

  Compare two subset files and display changes in test order positions.

Options:
  --help  Show this message and exit.

Example usage:

 % pr -m -t subset-before.txt subset-after.txt 
src/test/java/example/DivTest.java  src/test/java/example/Add2Test.java
src/test/java/example/DB1Test.java  src/test/java/example/MulTest.java
src/test/java/example/MulTest.java  src/test/java/example/AddTest.java
src/test/java/example/Add2Test.java src/test/java/example/File1Test.jav
src/test/java/example/File1Test.jav src/test/java/example/DivTest.java
src/test/java/example/File0Test.jav src/test/java/example/File0Test.jav
src/test/java/example/SubTest.java  src/test/java/example/DB1Test.java
src/test/java/example/DB0Test.java  src/test/java/example/DB0Test.java
src/test/java/example/AddTest.java  src/test/java/example/SubTest.java

% launchable compare subsets subset-before.txt subset-after.txt
|   Before |   After |   Order Change | Test Path                            |
|----------|---------|----------------|--------------------------------------|
|        9 |       3 |             -6 | src/test/java/example/AddTest.java   |
|        2 |       7 |             +5 | src/test/java/example/DB1Test.java   |
|        1 |       5 |             +4 | src/test/java/example/DivTest.java   |
|        4 |       1 |             -3 | src/test/java/example/Add2Test.java  |
|        7 |       9 |             +2 | src/test/java/example/SubTest.java   |
|        3 |       2 |             -1 | src/test/java/example/MulTest.java   |
|        5 |       4 |             -1 | src/test/java/example/File1Test.java |
|        6 |       6 |             +0 | src/test/java/example/File0Test.java |
|        8 |       8 |             +0 | src/test/java/example/DB0Test.java   |

When there're new test paths in the subset-after.txt, new file paths will be a marked as 'NEW':

% pr -m -t subset-before.txt subset-after.txt 
src/test/java/example/SubTest.java  src/test/java/example/NewTest.java
src/test/java/example/DivTest.java  src/test/java/example/SubTest.java
src/test/java/example/Add2Test.java src/test/java/example/File0Test.jav
src/test/java/example/File0Test.jav src/test/java/example/DB1Test.java
src/test/java/example/AddTest.java  src/test/java/example/DivTest.java
src/test/java/example/File1Test.jav src/test/java/example/MulTest.java
src/test/java/example/MulTest.java  src/test/java/example/File1Test.jav
src/test/java/example/DB0Test.java  src/test/java/example/DB0Test.java
src/test/java/example/DB1Test.java  src/test/java/example/Add2Test.java
				                    src/test/java/example/AddTest.java

% launchable compare subsets subset-before.txt subset-after.txt
| Before   |   After | Order Change   | Test Path                            |
|----------|---------|----------------|--------------------------------------|
| -        |       1 | NEW            | src/test/java/example/NewTest.java   |
| 3        |       9 | +6             | src/test/java/example/Add2Test.java  |
| 9        |       4 | -5             | src/test/java/example/DB1Test.java   |
| 5        |      10 | +5             | src/test/java/example/AddTest.java   |
| 2        |       5 | +3             | src/test/java/example/DivTest.java   |
| 1        |       2 | +1             | src/test/java/example/SubTest.java   |
| 4        |       3 | -1             | src/test/java/example/File0Test.java |
| 7        |       6 | -1             | src/test/java/example/MulTest.java   |
| 6        |       7 | +1             | src/test/java/example/File1Test.java |
| 8        |       8 | +0             | src/test/java/example/DB0Test.java   |

When there're deleted test paths between the two subsets, deleted test paths will be marked as 'DELETED':

pr -m -t subset-before.txt subset-after.txt
src/test/java/example/NewTest.java  src/test/java/example/DB1Test.java
src/test/java/example/SubTest.java  src/test/java/example/DB0Test.java
src/test/java/example/File0Test.jav src/test/java/example/File1Test.jav
src/test/java/example/DB1Test.java  src/test/java/example/SubTest.java
src/test/java/example/DivTest.java  src/test/java/example/AddTest.java
src/test/java/example/MulTest.java  src/test/java/example/MulTest.java
src/test/java/example/File1Test.jav src/test/java/example/File0Test.jav
src/test/java/example/DB0Test.java  src/test/java/example/Add2Test.java
src/test/java/example/Add2Test.java src/test/java/example/DivTest.java
src/test/java/example/AddTest.java

% launchable compare subsets subset-before.txt subset-after.txt
|   Before | After   | Order Change   | Test Path                            |
|----------|---------|----------------|--------------------------------------|
|        1 | -       | DELETED        | src/test/java/example/NewTest.java   |
|        8 | 2       | -6             | src/test/java/example/DB0Test.java   |
|       10 | 5       | -5             | src/test/java/example/AddTest.java   |
|        7 | 3       | -4             | src/test/java/example/File1Test.java |
|        3 | 7       | +4             | src/test/java/example/File0Test.java |
|        5 | 9       | +4             | src/test/java/example/DivTest.java   |
|        4 | 1       | -3             | src/test/java/example/DB1Test.java   |
|        2 | 4       | +2             | src/test/java/example/SubTest.java   |
|        9 | 8       | -1             | src/test/java/example/Add2Test.java  |
|        6 | 6       | +0             | src/test/java/example/MulTest.java   |

@gayanW gayanW requested review from kohsuke and ono-max August 14, 2025 06:56
changes.sort(key=lambda x: (abs(x[3]) if isinstance(x[3], int) else float('inf')), reverse=True)

# Display results in a tabular format
headers = ["Before", "After", "Order Change", "Test Path"]
Copy link
Contributor

@ono-max ono-max Aug 15, 2025

Choose a reason for hiding this comment

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

Probably, "Order Change" sounds a little bit unclear to me. Can we rename it to the following names?

  • After - Before
  • Order Δ
  • Order Diff
  • Diff (After - Before)
  • Δ (After - Before)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change it to 'After - Before'

after_tests = f.read().splitlines()
after_index_map = {test: idx for idx, test in enumerate(after_tests)}

changes = []
Copy link
Contributor

@ono-max ono-max Aug 15, 2025

Choose a reason for hiding this comment

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

Can you please define the type for this variable?

e.g.

DEFAULT_TIMEOUT: Tuple[int, int] = (5, 60)
DEFAULT_GET_TIMEOUT: Tuple[int, int] = (5, 15)

@kohsuke
Copy link
Contributor

kohsuke commented Aug 15, 2025

With this command, we are trying to draw attention to tests that boosted in ranks. So it is critical that we sort the result like that. IOW, sort them in the ascending order of diff.

@kohsuke
Copy link
Contributor

kohsuke commented Aug 15, 2025

"Test path" is a technical term in our product to refer to a certain structured test name, so I think it's best to avoid that, since what we are showing is not that.

Maybe just "test"?

@gayanW
Copy link
Contributor Author

gayanW commented Aug 15, 2025

"Test path" is a technical term in our product to refer to a certain structured test name, so I think it's best to avoid that, since what we are showing is not that.

Maybe just "test"?

I think we can name it 'Test'

Btw these are the names used in inspect subset command

header = ["Order", "Test Path", "In Subset", "Estimated duration (sec)"]

if test in before_index_map:
before_idx = before_index_map[test]
order_diff = after_idx - before_idx
changes.append((test, before_idx + 1, after_idx + 1, order_diff))
Copy link
Contributor

Choose a reason for hiding this comment

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

May I ask why you're using tuple, instead of map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think tuple is a suitable choice for representing a fixed-size row in a table. It should be also slightly more efficient compared to a map in this case.

I declared the variable with types, and also added a comment to make it clear on what goes in the tuple.

@gayanW
Copy link
Contributor Author

gayanW commented Aug 15, 2025

I updated the PR so that the results are sorted in ascending order of the diff value.
And changed the column name to 'Test'.

% launchable compare subsets subset-before.txt subset-after.txt         
|   Before |   After |   After - Before | Test                                 |
|----------|---------|------------------|--------------------------------------|
|        9 |       3 |               -6 | src/test/java/example/AddTest.java   |
|        4 |       1 |               -3 | src/test/java/example/Add2Test.java  |
|        3 |       2 |               -1 | src/test/java/example/MulTest.java   |
|        5 |       4 |               -1 | src/test/java/example/File1Test.java |
|        6 |       6 |               +0 | src/test/java/example/File0Test.java |
|        8 |       8 |               +0 | src/test/java/example/DB0Test.java   |
|        7 |       9 |               +2 | src/test/java/example/SubTest.java   |
|        1 |       5 |               +4 | src/test/java/example/DivTest.java   |
|        2 |       7 |               +5 | src/test/java/example/DB1Test.java   |

Copy link
Contributor

@kohsuke kohsuke left a comment

Choose a reason for hiding this comment

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

Woot!

@kohsuke kohsuke merged commit 4e46793 into main Aug 15, 2025
13 checks passed
@kohsuke kohsuke deleted the AIENG-217 branch August 15, 2025 15:18
@github-actions github-actions bot mentioned this pull request Aug 14, 2025
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