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.

Sign up for GitHub

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

Jump to bottom

Manage Apalache server #1115

Merged
merged 15 commits into from
Aug 25, 2023
Merged

Manage Apalache server #1115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
265d606
Move connection attempt into `connect()`
thpani Aug 21, 2023
2db5feb
Manage Apalache server
thpani Aug 23, 2023
10d7ee7
Bump tagged Apalache version
thpani Aug 23, 2023
88affaf
Bump Node.js to v18 in GH workflows
thpani Aug 23, 2023
c5c67ba
Use `find` instead of `filter`
thpani Aug 24, 2023
1f4d6b2
Use `tar` instead of `tar-fs`
thpani Aug 24, 2023
1a6a1d5
Rework retry logic
thpani Aug 24, 2023
ce89570
Update integration tests
thpani Aug 24, 2023
5980f16
Handle error fetching GH releases
thpani Aug 24, 2023
97bcb15
Update changelog
thpani Aug 24, 2023
d2045ae
Clean up dependencies
thpani Aug 25, 2023
e78648f
Lift code out of async function
thpani Aug 25, 2023
154604d
Improve console output
thpani Aug 25, 2023
0e10594
Fix integration test
thpani Aug 25, 2023
c58464f
Handle ESRCH on process.kill()
thpani Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: npm install
- run: npm run format-check || (echo "Run 'npm run format'" && exit 1)

Expand All @@ -40,7 +40,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: npm install
- run: npm run format-check || (echo "Run 'npm run format'" && exit 1)

Expand All @@ -57,7 +57,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: npm install
- run: npm run compile
- run: npm test
Expand All @@ -73,7 +73,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: npm install
- name: Compile and update test fixtures
run: npm run generate
Expand All @@ -92,7 +92,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: cd ./quint && npm ci
- run: cd ./quint && npm run compile && npm link
- name: Ensure all dependencies are specified
Expand Down Expand Up @@ -139,7 +139,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: cd ./quint && npm ci
- run: cd ./quint && npm run compile && npm link
- name: Ensure all dependencies are specified
Expand Down Expand Up @@ -176,7 +176,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- run: cd ./quint && npm install
- run: cd ./quint && npm run antlr

Expand All @@ -190,7 +190,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v2
with:
node-version: "17"
node-version: "18"
- name: Install quint deps
run: cd ./quint && npm install
- name: Install yalc
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
### Changed

- The `verify` command now manages Apalache server, if no already running
instance is detected (#1115)

Comment on lines 10 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a feature add, not a breaking change, iiuc?

Suggested change
### Added
### Changed
- The `verify` command now manages Apalache server, if no already running
instance is detected (#1115)
### Added
- The `verify` command now manages Apalache server, if no already running
instance is detected (#1115)
### Changed

Copy link
Contributor Author

@thpani thpani Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Do we use these categories in a way that diverges from https://keepachangelog.com/?
They simple state

Changed for changes in existing functionality.

Whether this is a new feature, or modification of an existing one is largely up to interpretation. I'd treat it as a changed feature, as it does not expose additional core functionality. Rather, it improves UX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will merge this PR, since Igor wanted to send install instructions for the summer school.
LMK if I should follow up with an updated changelog.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. I take "changes for existing functionality" to entail breaking changes. Otherwise, none of the categories entail breaking changes, and I also don't know how you can change existing functionality without that possibly breaking previous expectation about the API. That said, I can see how disagreement of the interpretation here is possible.

Apparently there has been a long-onging debate about this, lol: olivierlacan/keep-a-changelog#41

Perhaps we should add a "Breaking changes" section?

I don't think my view is not widely enough shared to require any change to your entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see – yeah, it looks like the keepachangelog sections are a bit orthogonal to indicating breaking changes. Incidentally, also Removed may indicate a breaking change, so I guess in this taxonomy breaking changes are scattered across sections.

If we want to list breaking changes, we can either prefix individual items with a qualifier or break them into a separate section, as you suggested.

### Deprecated
### Removed
### Fixed
Expand Down
105 changes: 23 additions & 82 deletions quint/apalache-dist-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,96 +4,37 @@
bash -
-->

These tests check for error handling around misconfiguration or corruption of
the apalache distribution. They are in a separate file from `apalache-test.md`
since they presuppose a broken environment, whereas the apalache integration
tests assume a working environment.
These tests check management of the Apalache server from Quint.
They are in a separate file from `apalache-test.md` since they presuppose an
environment in which Apalache server has not been started.

### Inability to connect to the server downloads Apalache

### Problems with the Apalache distribution
There is no Apalache server running. This will try to load the proto file
dynamically, using gRPC reflection, recognize that the server is not available,
fetch a tagged Apalache release and spawn a server in the background.

#### Setting a Non-existent `APALACHE_HOME` produces an error
The second time we call Apalache, we already have an up-to-date Apalache
distribution on-disk. Quint re-uses this distribution and does not download
again.

<!-- !test in invalid APALACHE_DIST -->
```
APALACHE_DIST=/does/not/exist quint verify ../examples/language-features/booleans.qnt
```

<!-- !test exit 1 -->
<!-- !test err invalid APALACHE_DIST -->
```
error: Specified APALACHE_DIST /does/not/exist does not exist.
```

#### Setting a corrupted `APALACHE_DIST` produces an error

(The `_build` dir exists, but doesn't have the expected structure of the
Apalache distribution.)

<!-- !test in corrupted APALACHE_DIST -->
```
APALACHE_DIST=_build quint verify ../examples/language-features/booleans.qnt 2> >(sed "s:$PWD/::g" >&2)
```

<!-- !test exit 1 -->
<!-- !test err corrupted APALACHE_DIST -->
```
error: Apalache distribution is corrupted: cannot find _build/lib/apalache.jar. Ensure the APALACHE_DIST environment variable points to the right directory.
```

#### Extracting the proto file from a corrupted jar file produces an error

We set up a valid looking distribution, but with an invalid jar file.

<!-- !test in corrupted apalache.jar -->
```
mkdir -p _build/corrupt-dist-test/lib && touch _build/corrupt-dist-test/lib/apalache.jar
mkdir -p _build/corrupt-dist-test/bin && touch _build/corrupt-dist-test/bin/apalache-mc
APALACHE_DIST=_build/corrupt-dist-test quint verify ../examples/language-features/booleans.qnt 2> >(sed "s:$PWD/::g" >&2)
```

<!-- !test exit 1 -->
<!-- !test err corrupted apalache.jar -->
```
error: Apalache distribution is corrupted. Could not extract proto file from apalache.jar.
```

### Inability to connect to the server produces an errer (`APALACHE_DIST` unset)

We now do not set `APALACHE_DIST`, which will try to load the proto file
dynamically, using gRPC reflection, but expect that the server is not already
running.

<!-- !test in server not running -->
```
quint verify ../examples/language-features/booleans.qnt
```

<!-- !test exit 1 -->
<!-- !test err server not running -->
```
error: Failed to obtain a connection to Apalache after 5 seconds.
```

### Inability to connect to the server produces an errer (`APALACHE_DIST` set)

We now assume a correctly configured installation of Apalache in
`APALACHE_DIST`, but expect that the server is not already running.

See https://github.com/informalsystems/quint/issues/823 for plans to manage the
server, making this error case all but impossible.

<!-- !test program
APALACHE_DIST=_build/apalache PATH=_build/apalache/bin:$PATH bash -
-->

<!-- !test in sever errors and connections -->
```
quint verify ../examples/language-features/booleans.qnt
quint verify ../examples/language-features/booleans.qnt | \
sed 's!https://.*!(asseturl)!'
quint verify ../examples/language-features/booleans.qnt | \
sed 's!\(Using existing Apalache distribution in \).*!\1(distdir)!'
```

<!-- !test exit 1 -->
<!-- !test err sever errors and connections -->
<!-- !test out server not running -->
```
error: Failed to obtain a connection to Apalache after 5 seconds.
Couldn't connect to Apalache, checking for latest supported release
Downloading Apalache distribution from (asseturl)
Launching Apalache server
Shutting down Apalache server
Couldn't connect to Apalache, checking for latest supported release
Using existing Apalache distribution in (distdir)
Launching Apalache server
Shutting down Apalache server
```
Loading
Loading