Skip to content

Conversation

@LironMShemen
Copy link
Collaborator

Identify the path of the currently running test across all test runners, so the cache can be saved there.

@asafkorem asafkorem changed the title Cache path fix: locate test file path Jun 25, 2025
Copy link
Collaborator

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!
Not sure about the async-await changes to the log function, I think we don't need them after your fix now.

error instanceof Error ? error.message : String(error);

logger.labeled("ERROR").error(`Execution failed: ${errorMessage}`);
await logger.labeled("ERROR").error(`Execution failed: ${errorMessage}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need this to be async-await now?

Comment on lines +40 to +49
// A wrapper function to make sure that failing test is not saved to cache.
// const wrapTest = (fn: () => Promise<void>) =>
// async function () {
// try {
// await fn();
// } catch (err) {
// testFailed = true;
// throw err;
// }
// };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

private cacheOptions?: CacheOptions;
private snapshotComparator: SnapshotComparator;
private codeEvaluator: CodeEvaluator;
private resolvedCacheFilePath?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a great idea

# Conflicts:
#	package-lock.json
The line this.cacheFilePath = this.determineCurrentCacheFilePath(); was added back to loadCacheFromFile() to handle cases like Detox.
Additionally, getCacheFilePath() was updated so that if getCurrentTestFileFromStackTrace() returns undefined, it first checks whether a path already exists before falling back to the default path.
After running in appium-test, still working.
*/
async perform(...steps: string[]): Promise<any> {
this.loadCache();
await this.loadCache();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check why do we need that

Comment on lines 361 to 374
const cacheFilePath = (newCacheHandler as any).cacheFilePath;
const cacheFilePath = await (newCacheHandler as any).cacheFilePath;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if needed

this.cacheFilePath = this.determineCurrentCacheFilePath();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need the await on this function

Comment on lines 299 to 300
progress: async () => {
await this.logWithLabel("info", label, "Starting");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably missed it

Copy link
Collaborator

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@asafkorem asafkorem merged commit beaccb3 into master Jul 6, 2025
3 checks passed
@asafkorem asafkorem deleted the cache-path branch July 6, 2025 11:57
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