-
Notifications
You must be signed in to change notification settings - Fork 5
Display captured exceptions in specs #563
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
Draft
floehopper
wants to merge
6
commits into
main
Choose a base branch
from
display-captured-exceptions-in-specs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously I was seeing many instances of the following deprecation warning when running `spec/features/admin/schools_spec.rb` which was cluttering up the build output making it hard to see genuine problems: DEPRECATION WARNING: The option :class_name is deprecated. Administrate should detect it automatically. This was because when the automatic associations functionality [1] was added to administrate in v0.15.0, the `class_name` option on `Administrate::Field::Associative#associated_class_name` which is the superclass for `Administrate::Field::BelongsTo`. The latter is used on the `SchoolDashboard` with the `class_name` option [3]. However, in the v1.0.0.beta3 release of administrate [4], this deprecation was reversed [4]. So we could avoid the deprecation by upgrading administrate. However, since it's a major version bump, it's like to be a chunk of work. So instead I've patched this specific functionality to avoid the deprecation warning. The patch includes code which will fail fast once administrate is updated to v1.0.0.beta3 or later so that the patch can be removed. [1]: thoughtbot/administrate#1633 [2]: https://github.com/thoughtbot/administrate/blob/f7287410a93d07f222bd5d4fb5055c6937eace08/CHANGELOG.md#0150-february-26-2021 [3]: https://github.com/RaspberryPiFoundation/editor-api/blob/f397e870f2a33cce1f53b9104c52314f5233572c/app/dashboards/school_dashboard.rb#L14 [4]: thoughtbot/administrate#2697
The `SchoolProject` was added in #490, but for some reason this spec was left with only a pending example. The latter is displayed in a warning list when you run the specs and constantlt makes me think something is wrong. To avoid this, I've implemented a couple of examples to test the most obvious behaviour of the `SchoolProject` model which seems better than nothing. These examples shouldn't be taken as an endorsement of the existing behaviour of or the desired behaviour of `SchoolProject`. For example, I wonder whether it should have some validations.
The example just before this one keeps failing in the CircleCI build [1], but I haven't been able to replicate it locally. It's hard to work out what's going on, because `SchoolTeacher::Invite#call` swallows any exception. I'm hoping that this new assertion _might_ fail for the same reason, but then display the exception message to give us more clues. [1]: https://app.circleci.com/pipelines/github/RaspberryPiFoundation/editor-api/2359/workflows/ecf1c252-3f22-45e6-b0fe-08363f66c8d9/jobs/4447
There are at least two flakey specs: * `spec/concepts/school_teacher/invite_spec.rb:14` [1] * `spec/concepts/project/create_remix_spec.rb:125` [2] I strongly suspect the extensive use of Faker in factories and spec setup and the randomness associated with it may be the root cause of the flakey specs. If nothing else it makes it hard to reproduce the spec failures locally. Configuring Faker's randomness with the same seed as RSpec is using and reporting should make it easier to reproduce any flakey specs in the future. [1]: https://github.com/RaspberryPiFoundation/editor-api/blob/f397e870f2a33cce1f53b9104c52314f5233572c/spec/concepts/school_teacher/invite_spec.rb#L14-L17 [2]: https://github.com/RaspberryPiFoundation/editor-api/blob/f397e870f2a33cce1f53b9104c52314f5233572c/spec/concepts/project/create_remix_spec.rb#L125-L128
I've reproduced this example failure locally as follows: bundle exec rspec --seed 58786 spec/concepts/project/create_remix_spec.rb:125 The problem was with the `component` factory using `Faker::Lorem.word` to generate `Component#name` [1] and then the example relying on the order of components which is partly based on `Component#name` [2]. And thus depending on the value of the generated name, `remixed_project.components.first` in the example didn't always refer to the component that the author intended and the assertion would fail, e.g. with the seed value of 58786, the original component had the name "ad" which comes before the new component name of "added_component" alphabetically. While I have some reservations about the use of Faker in spec setup, the simplest way to fix this is to find the new component by name before asserting against its attribute values. [1]: https://github.com/RaspberryPiFoundation/editor-api/blob/f397e870f2a33cce1f53b9104c52314f5233572c/spec/factories/component.rb#L5 [2]: https://github.com/RaspberryPiFoundation/editor-api/blob/f397e870f2a33cce1f53b9104c52314f5233572c/app/models/project.rb#L14
We still have at least one flakey spec [1] and I noticed it depends on the `verified_school` factory which uses the `ForEducationCodeGenerator.generate` method to set the `School#code`. On the offchance this is causing some kind of non-deterministic problem and to make that generator more deterministic in specs, I thought it would be worthwhile using the RSpec seed to seed the random number generator (RNG) used by `ForEducationCodeGenerator.generate`. The changes in this commit allow the RNG to be overridden and adds file in `spec/support` to do this in specs so it uses the RSpec seed. [1]: https://github.com/RaspberryPiFoundation/editor-api/blob/f397e870f2a33cce1f53b9104c52314f5233572c/spec/concepts/school_teacher/invite_spec.rb#L14-L17
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.