-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement or remove pending tests #4803
Merged
Merged
Conversation
This file contains 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
javierm
force-pushed
the
pending_tests
branch
5 times, most recently
from
April 4, 2022 12:56
ce1ab0e
to
e9d29b2
Compare
CONSUL doesn't implement blank votes via web; the comment was based on the code used in Madrid, which was actually very complex. And the concept of "all city" was also specific to Madrid. Poll questions aren't associated to a geozone, so the geozone will depend on the poll they're associated to.
The test "Sender email" already checks the receiver's name appears in the copy sent to the sender.
javierm
force-pushed
the
pending_tests
branch
3 times, most recently
from
April 7, 2022 11:34
492e9d1
to
8846c40
Compare
taitus
approved these changes
Apr 7, 2022
javierm
force-pushed
the
pending_tests
branch
2 times, most recently
from
April 7, 2022 11:55
b5038b4
to
e59da0b
Compare
The map feature was never implemented for debates (only for proposals and budget investments) and it was crashing for debates because the page didn't load the geozones. And we don't have a "geozone" field in the debates form either. So we're removing the map page alongside its (pending implementation) tests.
This feature was actually implemented, but the test was checking the wrong selectors.
So it works the same way as everywhere else.
We were skipping the test, but there's an identical test right below it.
This file only has tests related to tags; if the model doesn't have tags, we simply wouldn't include `it_behaves_as` in their tests instead of including it and then skipping it.
It looks like it was disabled because it was failing sometimes for some reason. I haven't found the reason, though; we're changing the test a little bit to make it easier to read. Enabling it will let us find out whether it still fails.
It was disabled in commit 792b15b for unknown reasons.
They were marked as pending. Note Capybara doesn't support finding a button by its `aria-labelledby` attribute, so we're using the ugly `click_button "Yes"`, like we did in commit fabe97e.
This feature was only enabled for proposals five years ago, and it hasn't changed since then. The pending test only gets in the way. Implement. Or implement not. There is no pending.
These tests are obsolete since commit 5ed308c.
We define the available locales in the test environment, so Spanish is always available in this environment even if it isn't available in the production environment.
This way the tests won't appear as "pending" when running the test suite, and so we get rid of a lot of noise in the test results. There doesn't seem to be a way to call `skip` without the test being marked as "pending". Note that in the globalizable tests we need to build a factory before deciding whether an atribute is required or not (particularly for the milestone factory, since milestone attributes are required depending on the presence of other attributes). This isn't possible before we're inside the test, so we can't add an `if:` condition to the test. So we're adding the condition inside the test instead. A minor inconvenience of this method is the test still runs even when the condition is `false`.
We were finishing the test with the first "visit", so it was doing nothing (other than potentially generating concurrency issues with other tests).
I'd say this feature is actually tested in the "proposal polls specific validations"; the empty test was probably added by accident in commit 4b8cc85.
Since we were creating a new answer in the form, we weren't getting the errors associated to the answer the administrator was trying to create, and so we were skipping the test. Using the answer which contains the information about validation errors fixes the issue and so we don't have to skip the tests.
The test is already working with poll question answers (which are the only ones using `has_many_images`) as well.
taitus
approved these changes
Apr 7, 2022
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.
Objectives