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

Add more tests #664

Merged
merged 2 commits into from
Jul 26, 2019
Merged

Add more tests #664

merged 2 commits into from
Jul 26, 2019

Conversation

iiroj
Copy link
Member

@iiroj iiroj commented Jul 21, 2019

This PR adds two tests.

1: Should clear unstaged changes when linter applies same changes

  1. There is a staged file that does not pass prettier’s formatting
  2. There are unstaged modifications that properly prettify the file
  3. lint-staged runs prettier --fix and git add to essentially do the same thing as the unstaged modifications

The result:

  1. lint-staged succesfully fixed and committed the file
  2. The unstaged modifications are gone, since they’re already in place

2: should fail when linter creates a .git/index.lock

  1. There is a staged file that does not pass prettier’s formatting
  2. There are further unstaged changes in the same file
  3. lint-staged runs prettier --fix and git add, but also create a .git/index.lock file, failing git operations (like the git add)

The result:

  1. lint-staged fails to create a new commit
  2. git status matches the status before committing, but prettier's changes have been staged
  3. The unstaged modifications are gone, replaced by previously staged modifications, and staged modifications contain linter changes.

@iiroj iiroj requested a review from okonet July 21, 2019 11:03
@codecov
Copy link

codecov bot commented Jul 21, 2019

Codecov Report

Merging #664 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #664   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files          11       11           
  Lines         305      305           
  Branches       57       57           
=======================================
  Hits          304      304           
  Misses          1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e879b6a...8f5b5b5. Read the comment docs.

@iiroj iiroj changed the title test: add test for prettier re-doing unstaged changes Add more tests Jul 21, 2019
@iiroj iiroj force-pushed the more-tests branch 2 times, most recently from 869c61d to 1d94a49 Compare July 21, 2019 12:23
@okonet
Copy link
Collaborator

okonet commented Jul 22, 2019

Is this mergeable? Shouldn’t the second test fail until we fixed the root cause?

@iiroj
Copy link
Member Author

iiroj commented Jul 22, 2019

@okonet I made the test to expect the failure result, so it succeeds. When the issue gets fixed, the test will start failing.

Given how the linter task creates a git lock, all git operations from there onwards will fail, so I doubt we can ever cleanly recover from it.

@okonet
Copy link
Collaborator

okonet commented Jul 22, 2019

Great! Let’s merge then. Can you rebase it please?

@iiroj
Copy link
Member Author

iiroj commented Jul 22, 2019

Alright, rebased.

@iiroj
Copy link
Member Author

iiroj commented Jul 25, 2019

I've rebased against latest master, but sadly have no permissions to merge myself.

@okonet
Copy link
Collaborator

okonet commented Jul 26, 2019

I think to solve that I’d need to create a separate organization. Thoughts?

@okonet okonet merged commit d9fcbda into master Jul 26, 2019
@iiroj iiroj deleted the more-tests branch July 27, 2019 08:28
@okonet
Copy link
Collaborator

okonet commented Aug 17, 2019

🎉 This PR is included in version 9.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants