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

Remove garnet spec #1306

Merged
merged 2 commits into from
Nov 15, 2022
Merged

Remove garnet spec #1306

merged 2 commits into from
Nov 15, 2022

Conversation

drujensen
Copy link
Member

@drujensen drujensen commented Nov 15, 2022

Description of the Change

This removes the Garnet Spec dependency that relied on selenium driver. This adds a RequestHelper set of macros that provide a harness to test controller tests. The macros create the http request methods to simplify specs.

This also enables the granite test suite which is crucial for a full set of test coverage for Amber.

Alternate Designs

Fix the garnet_spec and selenium driver.

Benefits

Although system or integration tests are valuable, the unit level tests shouldn't be dependent on this library. This separates the 2 concerns. You can still add integration tests using garnet (once its fixed) but they should be a separate set of tests and not tightly coupling the unit tests to these set of dependencies.

Possible Drawbacks

Moving the RequestHelper inside of a module instead of in its own shard may limit reuse of this functionality in other frameworks.

@drujensen drujensen requested review from crimson-knight and a team November 15, 2022 15:15
Copy link
Member

@robacarp robacarp left a comment

Choose a reason for hiding this comment

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

There is some drawback to community re-usability but I think I prefer a "wait until there's an obvious use case" for that kind of thing. The big crystal frameworks have a huge maintenance burden on them in releasing shards because of how much relentless tagging and pushing is required, and amber is no exception. Anything that lightens the maintenance burden is a win.

@drujensen drujensen merged commit fc0b60f into dj-remove-webpack Nov 15, 2022
@drujensen drujensen deleted the dj-remove-garnet-spec branch November 15, 2022 23:26
crimson-knight pushed a commit that referenced this pull request Nov 21, 2022
* fix: remove webpack from dependencies

This change removes webpack from the dependencies. It also upgrades
bootstrap to 5.0 using cdn links.

* fix: linting issue

* fix: specs

* fix: rollback default database

* fix: specs

* refactor: minor adjustments

* refactor: cleanup more tests

* fix: base64 spec

* refactor: avoid around_each for all specs

* fix: broken wrapper

* fix: fix REDIS_URL

* fix: make github action match docker compose

* fix: remove old crystal version file

* fix: update missing template changes

* fix: forcefully add public assets

* fix: remove cli from .gitignore

* fix: use latest images

* fix: remove nodejs from Dockerfile

* fix: remove node dependency

* fix: restore filter

* fix: missed in slang template

* fix: needs to be type module for ES6 support

* Remove garnet spec (#1306)

* fix: remove garnet_spec shard as dependency

* fix: specs

* 1227 fix layout equals false (#1307)

* fix: check for false

* fix: #1227 support LAYOUT=false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants