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

Fix intermittent CI build failures and add basic Angular functional tests #193

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

jamesdaniels
Copy link
Collaborator

@jamesdaniels jamesdaniels commented May 16, 2024

Something about the lru types is very broken ATM (see the build result of #190, #191, etc.), from local error messages it seems that they are passthrough now and not required... but that conflicts with the linked logs. Very flaky, regardless, this all seems to resolve on an NPM install and on CI we are skipping that on an NPM cache hit. IMO we should just call npm ci everytime right now, until we can figure out why the NPM cache step is insufficient / flaking.

While I was working on reproducing this I noted that the way we were packing adapter-angular/simple-server was also confusing things (see run #9114734835), so I took a pass on fixing that.

  • Force lru-types version bump
  • Make npm ci run even on npm cache hit
  • Changes to adapter-angular
    • Added some functional tests for the adapter both in SSR and non-SSR modes
    • simple-server
      • Convert to typescript
      • Drop it's own package.json and move deps to dev-deps
      • Add rollup to the adapter-angular's build script
      • SSG content was being served with the max-age header (see run #9119111443)

@jamesdaniels jamesdaniels marked this pull request as ready for review May 16, 2024 15:59
@jamesdaniels jamesdaniels changed the title Repro lru-break Fix the lru-types break May 16, 2024
@jamesdaniels jamesdaniels changed the title Fix the lru-types break Fix the intermittent firebase-frameworks and adapter-angular build failures May 16, 2024
Comment on lines +23 to +25
app.get("*", (request, response) => {
response.sendFile(path.join(__dirname, "..", "browser", "index.html"));
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since this is a static, single file, not dynamic we can probably ignore this if express caches; else we should read into memory.

@jamesdaniels jamesdaniels changed the title Fix the intermittent firebase-frameworks and adapter-angular build failures Fix the intermittent CI build failures and add basic Angular functional tests May 16, 2024
@jamesdaniels jamesdaniels changed the title Fix the intermittent CI build failures and add basic Angular functional tests Fix intermittent CI build failures and add basic Angular functional tests May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants