Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fixes the Open Line tests when the indentation is set as tabs in the main window #4805

Merged
merged 2 commits into from
Aug 21, 2013
Merged

Fixes the Open Line tests when the indentation is set as tabs in the main window #4805

merged 2 commits into from
Aug 21, 2013

Conversation

TomMalbran
Copy link
Contributor

I was running the test when I saw some fails in the Open Line tests. After some investigation I found out that it was because I had Tabs set as my indentation. So I fixed the tests so that they pass when Spaces or Tabs are set in the main editor.

@peterflynn
Copy link
Member

I think it would be better to have the unit test force the specific preferences configuration it's expecting. With this change, the tests will test different things on different peoples' computers, which could be very confusing if only one version is failing.

Maybe in general tests that run in the Jasmine window should be isolated from the main Brackets prefs in the same way that tests popping up a separate window are...

@TomMalbran
Copy link
Contributor Author

@peterflynn Makes sense. I updated the fix by setting the use of spaces and the space units when creating a mock editor, and used constants for all this stuff.

@ghost ghost assigned dangoor and redmunds Aug 19, 2013
@redmunds
Copy link
Contributor

This looks good.

The only question I have is about testing with the different indentation settings. It looks like all tests are now run with indentation set to 4 spaces. Seems like we should have some tests with Tabs and other lengths of spaces. Do we have them somewhere else and I am not seeing them? If not, are they appropriate here? If not, I'm OK with creating an issue so they get done at a later time.

Done with review.

@TomMalbran
Copy link
Contributor Author

All the tests are only done with 4 spaces indentation (specially now that we set it for that instead of using the editor settings). Open line creates a new lines and uses the CodeMirror APIs to indent it. So not sure if it needs more tests with other indentations, but if it does you can open a new issue for that.

@redmunds
Copy link
Contributor

Looks good. Merging.

redmunds added a commit that referenced this pull request Aug 21, 2013
Fixes the Open Line tests when the indentation is set as tabs in the main window
@redmunds redmunds merged commit ac7d6c3 into adobe:master Aug 21, 2013
@TomMalbran TomMalbran deleted the tom/open-line-fix branch August 22, 2013 01:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants