From 9d90cb7ff9fdc68fe0c194596cd71752733b7569 Mon Sep 17 00:00:00 2001 From: Nathan Houle Date: Fri, 18 Oct 2024 17:30:35 -0700 Subject: [PATCH] fix: remove duplicative integration install step In 0cfe6d8 I introduced a duplicative integration install step when `context=dev`, not realizing that we have a special case for resolving integrations in dev mode: https://github.com/netlify/build/blob/main/packages/build/src/plugins/resolve.js#L189-L195 Returning the tarball location meant we were installing the integration's packed tarball and then also installing from the pre-pack build directory. This changeset removes that duplication. It's... weird that we don't just return the location of a packed npm package (tarball) and instead have a special case that points at the pre-pack build artifact directory. This sort of special-cased action at a distance makes it super hard to understand how this works. I'm going to circle back on de-confusing this sometime this quarter when I make the extension build artifact path configurable, which will solve a lot of testing pain points we currently have. The failing test I also fix in this changeset was never realistic and didn't exercise some of the code paths it was supposed to put under test, so I've updated that test fixture to be realistic. --- packages/build/src/install/missing.js | 2 +- .../integration/.ntli/build/.gitkeep | 0 .../integration/.ntli/build/index.js | 3 +++ .../integration/.ntli/build/manifest.yml | 2 ++ .../integration/.ntli/build/package.json | 7 +++++++ .../integration/manifest.yml | 2 +- .../integration/package.json | 2 +- .../local_missing_integration/netlify.toml | 2 +- .../build/tests/install/snapshots/tests.js.md | 13 ++++++++++--- .../tests/install/snapshots/tests.js.snap | Bin 2534 -> 2534 bytes packages/build/tests/install/tests.js | 11 ++--------- 11 files changed, 28 insertions(+), 16 deletions(-) delete mode 100644 packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/.gitkeep create mode 100644 packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/manifest.yml create mode 100644 packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/package.json diff --git a/packages/build/src/install/missing.js b/packages/build/src/install/missing.js index cf7daf08e6..45c60b810b 100644 --- a/packages/build/src/install/missing.js +++ b/packages/build/src/install/missing.js @@ -98,7 +98,7 @@ const getIntegrationPackage = async function ({ throw new Error(`Failed to build integration. Error:\n\n${e.stack}`) } - return resolve(integrationDir, '.ntli/site/static/packages/buildhooks.tgz') + return undefined } return undefined diff --git a/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/.gitkeep b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/index.js b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/index.js index e69de29bb2..8c1f901340 100644 --- a/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/index.js +++ b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/index.js @@ -0,0 +1,3 @@ +export const onPreBuild = function () { + console.log("Hello world"); +} diff --git a/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/manifest.yml b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/manifest.yml new file mode 100644 index 0000000000..bbc1fee978 --- /dev/null +++ b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/manifest.yml @@ -0,0 +1,2 @@ +name: abc-integration +inputs: [] diff --git a/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/package.json b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/package.json new file mode 100644 index 0000000000..507b311db0 --- /dev/null +++ b/packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/package.json @@ -0,0 +1,7 @@ +{ + "main": "index.js", + "type": "module", + "version": "0.0.0", + "name": "abc-integration", + "dependencies": {} +} diff --git a/packages/build/tests/install/fixtures/local_missing_integration/integration/manifest.yml b/packages/build/tests/install/fixtures/local_missing_integration/integration/manifest.yml index a3512f0259..bbc1fee978 100644 --- a/packages/build/tests/install/fixtures/local_missing_integration/integration/manifest.yml +++ b/packages/build/tests/install/fixtures/local_missing_integration/integration/manifest.yml @@ -1,2 +1,2 @@ -name: test +name: abc-integration inputs: [] diff --git a/packages/build/tests/install/fixtures/local_missing_integration/integration/package.json b/packages/build/tests/install/fixtures/local_missing_integration/integration/package.json index 8521f0f9aa..26fafe3703 100644 --- a/packages/build/tests/install/fixtures/local_missing_integration/integration/package.json +++ b/packages/build/tests/install/fixtures/local_missing_integration/integration/package.json @@ -1,5 +1,5 @@ { - "name": "plugin_deps_plugin", + "name": "abc-integration", "version": "0.0.1", "type": "module", "scripts": { diff --git a/packages/build/tests/install/fixtures/local_missing_integration/netlify.toml b/packages/build/tests/install/fixtures/local_missing_integration/netlify.toml index c7204d2e8c..45a6d67bac 100644 --- a/packages/build/tests/install/fixtures/local_missing_integration/netlify.toml +++ b/packages/build/tests/install/fixtures/local_missing_integration/netlify.toml @@ -1,5 +1,5 @@ [[integrations]] -name = "abc-integration" +name = "test" [integrations.dev] path = "./integration" diff --git a/packages/build/tests/install/snapshots/tests.js.md b/packages/build/tests/install/snapshots/tests.js.md index 9b23f807b5..5f3ce01703 100644 --- a/packages/build/tests/install/snapshots/tests.js.md +++ b/packages/build/tests/install/snapshots/tests.js.md @@ -1051,7 +1051,6 @@ Generated by [AVA](https://avajs.dev). debug: true␊ repositoryRoot: packages/build/tests/install/fixtures/local_missing_integration␊ testOpts:␊ - cwd: ./tests/install/fixtures/local_missing_integration/␊ pluginsListUrl: test␊ silentLingeringProcesses: true␊ ␊ @@ -1070,10 +1069,18 @@ Generated by [AVA](https://avajs.dev). dev␊ ␊ > Building integrations␊ - - abc-integration from ./integration␊ + - test from ./integration␊ ␊ > Loading integrations␊ - - abc-integration␊ + - test␊ + ␊ + test (onPreBuild event) ␊ + ────────────────────────────────────────────────────────────────␊ + ␊ + Hello world␊ + ␊ + (test onPreBuild completed in 1ms)␊ + Build step duration: test onPreBuild completed in 1ms␊ ␊ Netlify Build Complete ␊ ────────────────────────────────────────────────────────────────␊ diff --git a/packages/build/tests/install/snapshots/tests.js.snap b/packages/build/tests/install/snapshots/tests.js.snap index 8828ee6096bd6dfb00e898a6dedc1ef40ea1da35..490600bcfb5a5706ec8e9f10fdfbf6b8be9f31bc 100644 GIT binary patch literal 2534 zcmV%-fYmtbk#>wM79lnRkH=%Iv0l$W z87EjchR_mQQrpdLqPwXXVPPP7g&;4mIp+;>5Ap&@E;$9rDQ}Qd(9Iu_8qMg3BE^+k z;K3Hz#pLy_l=t7tKl}+RqA0%l_-V-cif|@fAWSMmDR?-*%)_itumpys z!XA+f1#-ld{QY;HhZOqe!>@mM=k6W-`N5CxeDuN5ht3PEC>c!PaY(2KtA3n+|M7R< z>%(!5;MZ74!kGi`h(*?D_lz|TTc5T*)mJ>FXeb>35BI{M3rd980T4Lgk|-`F9nO^t z0qVa+LoB1wZH1+h?bto-0l8A4z_RTT;Q;89XMvKgqsOdkRc2R#j6Z$$EIp2?o%zN zgC(b9>_OiMbO4Nqc!FV%5;qzm7^bD|Z_<9nyGGI~X~a-o^7m`Kh+D*7P4^Ff~r+>8ya@Rim+5ZnvRWsb9) z0NCg3T;M23@fb68e^=t~Su{<;hEuQRl9}V|FKMKVjr!aVC|1}5!r;)C_nqi_sc-;Z zD3BiRLUF;4ljdURh0HS(dA~B~xrqin@2M#Ug8Ny2reW&9LHd20Fb`j~-blxB_UD*! zNhdBb(wV`zllc655G6l<5YI8W2jnw*BgN^9mvmB%n z-;~Kg_7^=9xg=@?Eb!+rk8zH93@!7hoOz_d*=2!W5B`x?X-yWA6H+P{nYCakj3hrD z%S&o=*yku)cR^tfuFPEQ6C~Is zNK8}f)7%t|F+Uws(!Mgy~7!z^>{IrYO^MrNye8-VlpNxg}bDam{OJ-nS{k5i$iri%Eh5C z*Tx~(HQ2yeHYT7`wwHUBE(cvHQjYI5iXXv6N`21NJzD zo5Y+zB!p5MxZxu;IzZ!L1A3tXH9~5NEE5=^2y}WFv&5R%g9%nGeW%BhB&;6f@hEN? zK-ccvl+egF7c>9~Y;J)<#eJlRc|$(ESIYso7-50Ug%XdkKol;~M1tZlAXlJ9MC$nI zQS2v=yE@qCV?DnK55;XaXC5HW(^sky&d)({^IahL8{Ah-{Y7v<@fg!tDOy>KycS$& zRfsg>21x3hU~VDNK>t&68fraSiIyQ_#tAI&lNmRF3ux#kdX2=2W)9a~$f6UyghKd; zd5PrX>6`|fQhurb7vYJXml@c|X5@ye{rT!uj5)7gPZK@-5*o`rPimMoVe}awq8Ex` zA%{`uxiP@4lqJ(9BRK7J&N?of@Q`|D!H5VzGD}$B{(Xm)ggd_45O;h4K4O#Hh&go9 zd7Gn?znv`%R{iWi9^M+A)c&d|r_8BVlvrkkRl_c4EL*o+kTZzonTtcz^2`FpTa9Pt znU!XlBQ(Z0a7#CX=?|bGM@R$>%Qr3GT#j!(j6zT@LpbN>%+*+DnRl74eT!(b6!+0Q ziW>;R6kH;qSGt7D5oXzf_7X|(i43rg^*L|rzRK~~FYMd8>buwz?{qyJ)Puj~Ra&>B zJu9T#zV;almebvyn~r6hiv!8N#;fg7?rXd+*2F2(Ei?zCKF?v)zQw4@G0J$RKKL|M zXH9HM1(b?QS*w+TNz-XqJX#zy7LRIsl#557#6L}r046EZv(F;jTejQS`+E+4{%YZ; za`>suCL0B|)+l>JjfLB2Y`8(f#L`T2#9ESDvBb8I2s7FCCImZzOpie}mS_45j2836 zKIh1rKB&9l!bfHaN63(O*zA=zHFNB2+#j|+bDaHaW>~#lV-*n=97_eqpDzoJlUp}* z*!yn|CI4knvT~H%)HV*W5~0M82}8OXDGr=PHghO_3x~xUH(2bn*jX?eSnS-6-Amm_ zdL`_)p7O)oBDxiuD0+!kqz#^hwQ3jQxkLPAjiYV=vX?Ic}oNny48MFBLVX_OhsHQM0@} zU{P~BcHeeg%6RUN9?jX=?p_Wv|7Yvc>fd|vZhT=qU^Ao7x^F>K3FW>F%^GgU@v|TR zo=3@{F*`zcM<^^9qPI2p>_AXo@9UYiB@jR0LdB+a(@S5Sb~@d!&z`(|e%gI;cF}!$ z_VUG(*DjnOrbi73^U~&yX3{s&jHO{Oa>L32GrJ@J&XdHh5TV-h=g9(!KiAd~BpEEd zBuZSm-6F|Q(WxO(pNy0Wq}y%}i5i8yR-dOoVThD&4T*|}3I#;UQ0Ey~G^`z6IW@hG z8x^J1-;4;wK;tz%XH{Nb!oRvWKKTkRI>#rcQ;~t-0TwEvj^9GxJPd5k^IH?%ZMxlL z*V0eID?VS2R|u|u?|Un`*!w6)EBa&hJAuUNY7K){&QJ6pQ?8>n3u$gst0sm1H@5+LIfDKj0ngymvm&O6qbTGqnN zmRd++$JL{p&8t+Xl_VBYK|9IRf~7&Dtv;&@!+KYr`Ndsr0UE*ku9k>f3+4o7Uq53e z#)c6(gNMiFCo>PyO|$ZEWAru~$r-)vMRu%d<{WU**A6k$o>TF4au9rCA$TPS?nn7o zuZwXRh}_NivxexLQ*RSUofTLeYR_2t#-PsuuJ*Wd6opB&V_hmE##^%wj>&qh_Kcwt we4aljDxy|<=ec7+_$8*4!zCAbhqirVj@9eOMZ;{#SW3zGf5r!U-)fEk0Bl&=82|tP literal 2534 zcmVzpvg_I3iPUP(5s#q{*I(w{ZOPBc?4py zOLESfIe*_bq-OSqVc(^L*X&P!!QA04zI^|2NCw=Ygqg@8j9adYZ0uuVV={0sLj*I9 zZHE!dXJg9Q_dnRS#jUU3{py!DZru={U);R$-aGf-HJ)MaI>RY?7&@+v*8Ld&{_}S~ z>%%baqu=9zfh`S$9+1cyoxZfjQTwCzN8*mht~FwC-NyZJWFj7f82$>dPnpAMFzr&x zP2^jH*VYKLsC9>f4_GI5PiN>{@-Tplwi7^Gaq-OO%rt}fWT;flnk8_8bVL&H0{KuYIEwj zLQWSm>Q1nY22vo%k|N>(hJDvznrXyDa$2eAR@`%IKm**24bAbT(B2QI9m?Ab zV>JQjkdpHNM@fn&;P5;9l7CO4VM26gUR8>jVH~b#q@0Zg)bm{oH4r?Cjy!h9h|WV< z^pPC~mS}FG;(`q$t;G=V#FhhjyDI3p%my*;%oM|bdU=1QX=*@{CFdbKZG74O28?JN zE->SYPF$g-F@tj_@%6XJONXijRp5?`v*SIX8)YQxATUD6=6)iJO2l%2bvum{&>F8c9Fb=8eK(h~_XmQ|Qy&7LAcw73MKQN<~B_He<<5Mh-|*nc5th%Iw?)g*~XLOl^;HsZ3{eDgzBq zo-80Te^ZFeUp0{_KxDx5E}cHHO{C&oZ{WaHkJ{uVogZdDdug_)5B?!R(DlA zikA#nb3Hd1YUCFeG>{cw*@YECWdKjfH{@AgZRqCt7{g8CMusOiuv~OuO&Q`8O423h z45NYk>AToV9(Q%IM<-%@QyK=>bxv$#*|xZokFh!i5HHSw?S>W;H9>a4tfBZ73SKDmXeU~Rgh(e4-%Cc^K;XNEc%s)xtSHAY??Mt? z=qD7ygPsz}$Jv;M)OG2F_$$H_F)rD#k*~-NQ~2}c%NTQBy~+wb`W!V@d!E#gF(LIC zT8;=5$wH2z)H7p%TdPW@i;U1|w|mw#p=5?2UIa`K0Z1-{MfYzztR&p=<(9bPT?BD8 zRaz{dlg4X>PX2LrVzBOK59;vF=%nyhO*y5aT2W$|7gi0soUv@(YJp-9%`=y$sOFgk zjJF=oRGF1vnPY2$ui%zu4%6?V1{+&}-_U$h^Uc-x=DjEd*(!u{am`$hb(Yyn7W*2} zW-0E2MHJTu_QD}w0}<&AU5uf)4Dqo z!CzICHeJ!47gFxJ_8AM7v$|awj%J(76G^|utL;(lYrM}k#3{L2XaPojqF~ga#;D3M zN_wU~_>`%$AvUD~O2wtT)k?vnY#17kmM4wIquL(j;?alkSCb=vNv7%fXA$N#+ie{D zQ-PnqYxt=germJHMuDw0%HB|8={6c$ULYZJX=ZW6Mv~jH#Exe9P^st4u>Nw5bwp@zEEODo zyDB(Nuien$;J*q={!61|0hgmPbo z<_)*!_*oDD&!ghdm~A0eM{pbvs2|rX@Y#c;o?q9KElY4bp9VZOt(o5X^0eFSeRcNe z#naQ?v$N;D$7e5|J$hxL6N`veLxz>aMMw$UK=jNO~FbIW1*-s zXj!vPbZ2IIkvBXltA7|nIe`;VOfQJa`4Ily^W&2*(ev){$!R7Ma>U00kEr9fkY?gx zU~`e*n&{28mz(Tc`AK-iC#&&_fQs+?UP~?x-c!iMJ9T*W(pH#sdbCTX73V zJ~tM;W1X4HYPi|a5|Y?)^_b48DivBv5(}wdIZ0;0(x6e#KC4T^de1)9;I4K88o~R% zP7!yO%n8iCdO~E5jUsf0{wDG(Gk4QXv(&dSVw;WRirDrd-&QW>9MWJQ93qE355YG- zS0K23Q$z4N5Il(LuZW9D8Hn7=`Ew4P2N$*oq;=(VY69)b*tP-l${;I){23QrSuqr} w7U2RXWR6AK-K8>Ou(tYxRz8_!^=x-$Y?k2s()N0GRLbn=|4wp86IG1>0JOp1zyJUM diff --git a/packages/build/tests/install/tests.js b/packages/build/tests/install/tests.js index 343da99ad0..3e69408b17 100644 --- a/packages/build/tests/install/tests.js +++ b/packages/build/tests/install/tests.js @@ -153,15 +153,8 @@ test('Install local plugin dependencies: missing plugin in netlify.toml', async t.snapshot(normalizeOutput(output)) }) -test('In integration dev mode, install local plugins and install the integration when forcing build', async (t) => { - const output = await new Fixture('./fixtures/local_missing_integration') - .withFlags({ - context: 'dev', - testOpts: { - cwd: './tests/install/fixtures/local_missing_integration/', - }, - }) - .runWithBuild() +test.only('In integration dev mode, install local plugins and install the integration when forcing build', async (t) => { + const output = await new Fixture('./fixtures/local_missing_integration').withFlags({ context: 'dev' }).runWithBuild() t.snapshot(normalizeOutput(output)) })