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

aegir test is broken #975

Closed
SgtPooki opened this issue May 5, 2022 · 3 comments · Fixed by #1008
Closed

aegir test is broken #975

SgtPooki opened this issue May 5, 2022 · 3 comments · Fixed by #1008
Labels

Comments

@SgtPooki
Copy link
Member

SgtPooki commented May 5, 2022

Describe the bug
When running aegir test, it always attempts to build, but my project is set up to build prior to running test, so this results in duplicated builds.

Additionally, it is passing an invalid flag, --no-bundle to my build command for some reason

To Reproduce
Steps to reproduce the behavior:

  1. Run these commands in a *nix type shell.
git clone [email protected]:ipfs-shipyard/js-pinning-service-http-client.git --branch dependabot/npm_and_yarn/aegir-37.0.15
cd js-pinning-service-http-client
npm install && npm run test
  1. see that a build is requested via aegir build

Without passing --build flag

it should default to false, but it doesn't.

> aegir test --types false --progress true
build

> @ipfs-shipyard/[email protected] build
> run-s build:main && run-s post-build "--no-bundle"


> @ipfs-shipyard/[email protected] build:main
> aegir build

[10:43:18] tsc [started]
[10:43:19] tsc [completed]
[10:43:19] esbuild [started]
[10:43:19] esbuild [completed]
ERROR: Invalid Option: --no-bundle
Command failed with exit code 1: npm run build --if-present -- --no-bundle

With --build false

> aegir test --build false --types false --progress true
build

> @ipfs-shipyard/[email protected] build
> run-s build:main && run-s post-build "--no-bundle"


> @ipfs-shipyard/[email protected] build:main
> aegir build

[10:38:19] tsc [started]
[10:38:20] tsc [completed]
[10:38:20] esbuild [started]
[10:38:20] esbuild [completed]
ERROR: Invalid Option: --no-bundle
Command failed with exit code 1: npm run build --if-present -- --no-bundle

After updating build script to be "aegir build"

╰─ ✘ 1 ❯ npm run test                                                                                                            11.96   28.1G   19%   100%  ─╯

> @ipfs-shipyard/[email protected] pretest
> tsc -p tsconfig.MockServerController.json


> @ipfs-shipyard/[email protected] test
> run-s test:*


> @ipfs-shipyard/[email protected] test:browser
> aegir test --target browser

.envrc not found
build

> @ipfs-shipyard/[email protected] build
> aegir build "--no-bundle"

.envrc not found
[11:04:44] tsc [started]
[11:04:45] tsc [completed]

> @ipfs-shipyard/[email protected] postbuild
> cp-cli dist.generated dist/dist.generated

test browser
⠋ Setting up chromium.envrc not found
✔ chromium set up

  Client
    ✅ Can be instantiated
    Operations
Failed to load resource: net::ERR_CONNECTION_REFUSED
http://localhost:3000/start
      1) "before each" hook for "GET: Can get failed Pins"
Failed to load resource: net::ERR_CONNECTION_REFUSED
http://localhost:3000/stop/3000
      2) "after each" hook for "GET: Can get failed Pins"
  1 passing (191ms)
  2 failing
  1) Client
       Operations
         "before each" hook for "GET: Can get failed Pins":
     TypeError: Network request failed
      at file:///fetch-ponyfill/build/fetch-browser.js:550:24

  2) Client
       Operations
         "after each" hook for "GET: Can get failed Pins":
     TypeError: Network request failed
      at file:///fetch-ponyfill/build/fetch-browser.js:550:24

Command failed with exit code 1: pw-test test/**/*.spec.{js,cjs,mjs} test/browser.{js,cjs,mjs} dist/test/**/*.spec.{js,cjs,mjs} dist/test/browser.{js,cjs,mjs} --mode main --config /private/var/folders/bl/_gl5_59s11v7qz5ysd6bfgb00000gn/T/tmp.LiAUXYuO/js-pinning-service-http-client/node_modules/aegir/src/config/pw-test.js --timeout=60000
ERROR: "test:browser" exited with 1.

Expected behavior

  1. Aegir should not try to build by default (the help output says that build defaults to false). So we should not call aegir build if we are not building...
  2. Aegir should respect --build false and -b false and not call aegir build if set to false.
  3. Aegir should not pass --if-present -- --no-bundle. If a build is requested, either:
    1. the user should be aware that a build script is necessary, because the user controls whether aegir is building or not (once this bug is fixed), so user is aware of whether they should have a build script or not.
    2. aegir should call aegir build directly, and not the npm run build version.
  4. Aegir should not assume that someone's npm build script starts and ends with aegir build.

Screenshots
N/A

Desktop (please complete the following information):

  • OS: macOS Monterey - 12.3.1 (21E258)

Additional context
This change of functionality is not called out in https://github.com/ipfs/aegir/blob/master/md/migration-to-v37.md

@achingbrain
Copy link
Member

I guess we could remove the build step(s) from the test command and require all projects to have a "pretest" script if they need to build before testing?

@SgtPooki
Copy link
Member Author

@achingbrain I think that makes more sense, but we could fix the --build flag too?

achingbrain added a commit that referenced this issue Jun 23, 2022
We run build before test but pass `--no-bundle` on the assumption
that a project build command is `aegir build`.  Passing `--no-bundle`
skips making a minified build for a tiny speedup.

If the project build command isn't `aegir build` this can break things
so remove the optimisation.

Also build by default before test and fix the problem whereby the build
flag was being ignored for typescript projects.

Fixes: #975
achingbrain added a commit that referenced this issue Jun 28, 2022
We run build before test but pass `--no-bundle` on the assumption that a project build command is `aegir build`.  Passing `--no-bundle` skips making a minified build for a tiny speedup.

If the project build command isn't `aegir build` this can break things so remove the optimisation.

Also build by default before test (to support typescript) and fix the problem whereby the build flag was being ignored for typescript projects.

Fixes: #975
github-actions bot pushed a commit that referenced this issue Jun 28, 2022
## [37.4.2](v37.4.1...v37.4.2) (2022-06-28)

### Bug Fixes

* remove no-bundle from test build and fix flag ([#1008](#1008)) ([f888be4](f888be4)), closes [#975](#975)
@github-actions
Copy link

🎉 This issue has been resolved in version 37.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants