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

Smoke Tests Bug Fixes #2094

Merged
merged 3 commits into from
Sep 2, 2024
Merged

Smoke Tests Bug Fixes #2094

merged 3 commits into from
Sep 2, 2024

Conversation

jonathanrainer
Copy link
Contributor

@jonathanrainer jonathanrainer commented Sep 2, 2024

We've seen some spurious smoke test failures recently because of being unable to decode the schema that comes back when its being watched. My hunch is that this is because we don't quite wait long enough in the test so this increases the timeout. To try and add some extra safety we also shutdown the watch command post-waiting so that the state of the file should be fixed before we attempt to read it. This should make this safer

This also includes a fix for the smoke tests so that Slack should actually get messaged when the workflow fails. Beforehand because it was relying on a state of the job looked up explicitly, if a job failed elsewhere it would cancel all other jobs so the messaging job would never run. However in this version and using the special failure() syntax we can run the messaging job whenever any task fails without it being skipped.

Green Smoke Test Run: https://github.com/apollographql/rover/actions/runs/10665653050

@jonathanrainer jonathanrainer requested a review from a team as a code owner September 2, 2024 10:28
At the moment we only wait 2 seconds, and we keep the introspection
command running for the whole test. We don't need to do the latter,
and from empirically running the smokes it seems as though we need
slightly more time to ensure writing to the file is complete.
At the moment we're scanning for a result that, while it exists, is
irrelevant because of the GitHub Actions does status conditions.

This should fix that problem so that this flow only runs on the
failure of the previous two steps, rather than being skipped as
happens at present.
This way the UI looks much prettier
@jonathanrainer jonathanrainer enabled auto-merge (squash) September 2, 2024 11:02
@jonathanrainer jonathanrainer merged commit 624feff into main Sep 2, 2024
22 checks passed
@jonathanrainer jonathanrainer deleted the jr/smoke-test-bug-fixes branch September 2, 2024 11:05
@jonathanrainer jonathanrainer added this to the v0.26.1 milestone Sep 4, 2024
@jonathanrainer jonathanrainer mentioned this pull request Sep 4, 2024
jonathanrainer added a commit that referenced this pull request Sep 4, 2024
# [0.26.1] - 2024-09-04

## 🚀 Features

- **Respect the use of `--output` flag in the supergraph binary -
@aaronArinder PR #2045**

In testing to attempt to reduce the runtime of `supergraph compose` we
noticed that a very large proportion of the time spent (in the case of
large supergraphs) was spent printing the result to `stdout`. With this
change we add an `--output` flag to the `supergraph` binary which means
this time can be reduced significantly, leading to much faster
compositions.

- **Add `--license` flag to `rover dev` - @loshz PR #2078**

Adds the ability to pass along an offline enterprise licence to the
router when running `rover dev`

- **Remove Rayon and reduce usage of Crossbeam - @jonathanrainer PR
#2081**
  
Now that `rover` has transitioned to using an asynchronous runtime we
don't need to use Rayon any more. This also resolves a bug whereby
`rover dev` could lock up if passed a `supergraph.yaml` file with lots
of subgraphs in.

- **Introduce new print macros - @loshz PR #2090**
  
Adds three new macros to the codebase so that we can still visually
distinguish between INFO, WARNING and ERROR log lines without the use of
emoji

- **Use new print macros in place of emoji - @loshz PR #2096**

Updates the locations that previously used emoji to utilise the new
macros defined in the previous PR

## 🐛 Fixes

- **Stop Windows Installer failing if whitespace is accidentally passed
to the `rover install` command - @jonathanrainer PR #1975**

In some situations it was possible for whitespace to be passed to the
`rover install` command which then caused the installer to fail. A guard
has now been added to strip whitespace out before it is passed to the
install command.

## 🛠 Maintenance

- **Move CI to using newly create Ubuntu images - @jonathanrainer PR
#2080**

CircleCI is removing support for older Ubuntu machine images, this
brings us up to date but does **not** change any of our `glibc` support
etc.

- **Add check for aarch-64-unknown-linux-musl to installers - @loshz PR
#2079**
- **Update node.js packages - @jonathanrainer PR #2070**

  Includes `eslint` to v9.9.1 and `node` to 20.17.0

- **Update `node` CircleCI orb to v5.3.0 - @jonathanrainer PR #2071**
- **Update `apollographql/federation-rs` to v2.9.0 - @jonathanrainer PR
#1983**
- **Update `apollographql/router` to v1.52.1 - @jonathanrainer PR
#2077**
- **Update `node` Docker Image to v20.17.0 - @jonathanrainer PR #2072**
- **Update `apollographql/router` to v1.53.0 - @jonathanrainer PR
#2084**
- **Update `npm` to v10.8.3 - @jonathanrainer PR #2091**
- **Update `slackapi/slack-github-action` to v1.27.0 - @jonathanrainer
PR #2092**
- **Update `node` CircleCI orb to v6.1.0 - @jonathanrainer PR #2093**
- **Fix some bugs in the smoke tests - @jonathanrainer PR #2094**

## 📚 Documentation

- **Add `cloud config` docs - @loshz PR #2066**
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants