Skip to content

Don't exclude server modules when checking impacted modules#14632

Merged
hashhar merged 1 commit intotrinodb:masterfrom
nineinchnick:impact-analysis-with-rpm
Nov 10, 2022
Merged

Don't exclude server modules when checking impacted modules#14632
hashhar merged 1 commit intotrinodb:masterfrom
nineinchnick:impact-analysis-with-rpm

Conversation

@nineinchnick
Copy link
Copy Markdown
Member

Description

If the server and/or rpm modules are impacted, the list of impacted modules should not be empty, because otherwise, it will cause all tests to run.

Non-technical explanation

n/a

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@nineinchnick
Copy link
Copy Markdown
Member Author

@hashhar PTAL

If the server and/or rpm modules are impacted, the list
of impacted modules should not be empty, because otherwise
it will cause all tests to run.
@nineinchnick nineinchnick force-pushed the impact-analysis-with-rpm branch from a59fc42 to a2ed5cf Compare November 10, 2022 09:24
Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Can you remind me what user visible effect we're trying to solve here?
IIRC the issue can be that if you change trino-server or trino-server-rpm then the -pl excluding them causes them to also be excluded from GIB's impacted list. Meaning that the impacted list becomes empty causing unnecessary tests to run?

It'll be good to mention that -pl impacts the GIB impacted list - it's not obvious from the commit message why those two would be related.

@nineinchnick
Copy link
Copy Markdown
Member Author

We usually exclude the server modules because they take a long time to build. But in the validate step they should not be excluded, because:

  • nothing is being built, so the original reason is not valid
  • we need all modules there to properly build a list of impacted modules, otherwise, the impacted modules list can be empty

If the impacted modules list is empty, in the build-test-matrix job we don't prune the matrix and run all test jobs to be on the safe side. But in every test job we check the impacted modules list again, and there are always only some modules selected. Since the impacted modules list and module selection exclude each other, there's nothing to do, tests for all modules are executed. So we end up doing too much, and some tests can fail, because of unexpected configuration.

@hashhar hashhar merged commit d599986 into trinodb:master Nov 10, 2022
@nineinchnick nineinchnick deleted the impact-analysis-with-rpm branch November 10, 2022 14:08
@github-actions github-actions bot added this to the 403 milestone Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants