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

using asar fiddle file #106

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

using asar fiddle file #106

wants to merge 2 commits into from

Conversation

hachimetsu
Copy link

#100
image
what improvement should i do
am i doing right ?

  • I used asar to unpack file 1st
  • and then used fromFolder function on extracted files
  • using temp dir

@hachimetsu hachimetsu requested a review from a team as a code owner March 16, 2024 11:21
@hachimetsu
Copy link
Author

hachimetsu commented Mar 16, 2024

i used my most of the time to just create test case for asar and still not done so i just created a pull request

i only done changes in src/fiddle.ts file
added the fromASAR function
which unpack asar content to tempDir and then call fromFolder on tempDir

@hachimetsu
Copy link
Author

@erickzhao can i also help i would like to learn how to create test case

@hachimetsu
Copy link
Author

hachimetsu commented Mar 21, 2024

image
what should i do in test case ?
image
I know how to make a test case pass, but then it doesn't behave like a real test case anymore.

A test case should show an error if there's one, but I'm unsure what conditions would constitute an error in this situation.

@hachimetsu
Copy link
Author

i did search about api-extractor i did not created a new pull because i want to compete the test case part too
image

@marfgold1
Copy link

A test case should show an error if there's one, but I'm unsure what conditions would constitute an error in this situation.

Do you mean a negative case? You can simply use not before condition, e.g. expect(fiddle!.mainPath).not.toBe(asarPath), but I don't think it's needed anyway? The test case could be structured like reads fiddles from gists test:

  • create the fiddle from asarPath
  • make sure the Fiddle object is created
  • make sure the mainPath of the fiddle is exist and has a basename of main.js
  • make sure the fiddle is kept in the fiddle cache

You can be more rigorous by making sure the file list and files' content are identical by comparing it with fiddleFixture('642fa8daaebea6044c9079e3f8a46390') just like in the reads fiddles from local folders test.

@hachimetsu
Copy link
Author

thanks for suggestion

For Case Running Fiddle from Local Dir

we test if mainPath in fiddle object is same as provided path
and there content are same or not

what should be done For ASAR case:-

asarPath: path to asar file (example fiddle-core/tests/fixtures/fiddle.asar)
mainPath: path to dir where content from asar get extracted (example fiddle-core-running-temp-path/extract/temp/fiddle)
for now what i did for test

  • provided path asarPath exist or not
  • extracted content have main.js
  • to make sure fiddle extract is in fiddle cache

NOTE

Should i compare extracted content to the content of dir which is used to create asar because i did use test example to create asar so this can be done

but then testing of asar will depend on content of example fiddle if in future fiddle example get changed then asar file must be create from that example

I m implementing all of this, so let me know if some one get any more idea or suggestion on this

@marfgold1
Copy link

but then testing of asar will depend on content of example fiddle if in future fiddle example get changed then asar file must be create from that example

Yeah that might happen. I have an idea, what about archiving the fiddleFixture('642fa8daaebea6044c9079e3f8a46390') as ASAR on the fly when testing it? Save it in tmpdir and let asarPath points to it. Then you can create fiddle fromASAR with it. Don't worry about cleaning up, afterEach(()=>{fs.removeSync(tmpdir);}) already done the cleaning job for you.

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