Skip to content

Commit

Permalink
fix(validate): validate to always validate impacted packages
Browse files Browse the repository at this point in the history
  • Loading branch information
azlam-abdulsalam committed Apr 19, 2024
1 parent 0cdaa37 commit 66d2da3
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
7 changes: 3 additions & 4 deletions decision records/validate/005-validate-to-skip-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ Initially, we considered adding a new flag and specific mode for this feature. H
This also means the following changes to behavior of how validate works

- Differentiate between packages that need to be synchronized vs validated
- Allow validate to commit the changed packages in to the target org, reverting earlier change to disableArtifactUpdate,so
users can control the behavior required, Ideally disableArtifactUpdate will be set to false, so retries to the same
review org within a range of commits will save further time
- Only commit the packages to an org, if the test pass for validate packages, however commit packages to org immediately for packages that are to be synchronized, when the deployment is succesful
- Allow validate to commit the changed packages in to the target org, reverting earlier change to disableArtifactUpdate
- Always validate impacted packages to ensure retries between runs are consistent and reduce confusions
- Only commit the packages to an org, if the test pass for validate packages, however commit packages to org immediately for packages that are to be synchronized, when the deployment is successful
- Package Diff comparison logic need to be updated to ensure, that if the commit id in the org is incorrect, assume the package is never built


21 changes: 16 additions & 5 deletions packages/sfp-cli/src/impl/parallelBuilder/BuildImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export default class BuildImpl {
await this.filterPackagesToBeBuiltByChanged(
this.props.projectDirectory,
this.packagesToBeBuilt,
this.props.impactedPackagesAsPerBranch
);
table = this.createDiffPackageScheduledDisplayedAsATable(
packagesToBeBuiltWithReasons,
Expand Down Expand Up @@ -333,7 +334,8 @@ export default class BuildImpl {

private async filterPackagesToBeBuiltByChanged(
projectDirectory: string,
allPackagesInRepo: any,
packagesInRepoFilteredByConfig: any,
impactedPackagesAsPerBranch: Map<string, string[]>
) {
let packagesToBeBuilt = new Map<string, any>();
let buildCollections = new BuildCollections(projectDirectory);
Expand All @@ -344,7 +346,7 @@ export default class BuildImpl {
this.props.currentStage,
);

for await (const pkg of allPackagesInRepo) {
for await (const pkg of packagesInRepoFilteredByConfig) {
let diffImpl: PackageDiffImpl = new PackageDiffImpl(
new ConsoleLogger(),
pkg,
Expand All @@ -353,11 +355,17 @@ export default class BuildImpl {
);
let packageDiffCheck = await diffImpl.exec();

if (packageDiffCheck.isToBeBuilt) {

let isPackageImpacted = impactedPackagesAsPerBranch
? impactedPackagesAsPerBranch.get(pkg)
: false;

if (isPackageImpacted || packageDiffCheck.isToBeBuilt) {
packagesToBeBuilt.set(pkg, {
reason: packageDiffCheck.reason,
reason: isPackageImpacted?'Found change(s) in package':packageDiffCheck.reason,
tag: packageDiffCheck.tag,
});

//Add Bundles
if (buildCollections.isPackageInACollection(pkg)) {
buildCollections
Expand Down Expand Up @@ -397,8 +405,11 @@ export default class BuildImpl {

//Filter Packages
if (includeOnlyPackages) {
//Display include only packages
//Display include only packages only on stages other than validate
//Validate would have already printed it
if(this.props.currentStage != Stage.VALIDATE){
printIncludeOnlyPackages();
}
packageDescriptors = packageDescriptors.filter((pkg) => {
if (
includeOnlyPackages.find((includedPkg) => {
Expand Down

0 comments on commit 66d2da3

Please sign in to comment.